Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Atomic replace patches will fail to load if previous patches targeted aliased functions. --
commit e5553650270f93e11fe885d264a3829802223d3a Author: Harry Hsu <[email protected]> livepatch: Fail object initialization on duplicate patched function This commit introduces a check to prevent patching multiple aliases of a function within a single livepatch. It fails object initialization if aliases resolving to the same address are detected. > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index a240d1144e89c..a6762cbe74b70 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c [ ... ] > @@ -885,6 +885,21 @@ static int klp_init_object_loaded(struct klp_patch > *patch, > if (ret) > return ret; > > + /* > + * Aliased symbols share one address, so they would resolve to > + * the same klp_ops and stack up on a single ops->func_stack, > + * leaving the redirection ambiguous. > + */ > + klp_for_each_func(obj, prev_func) { > + if (prev_func == func) > + break; > + if (prev_func->old_func == func->old_func) { > + pr_err("'%s' and '%s' resolve to the same > address, aliased symbols are not supported\n", > + prev_func->old_name, func->old_name); > + return -EINVAL; [Severity: High] Will this check inadvertently reject nop functions generated by an atomic replace patch when previous patches targeted aliased functions? If previously loaded livepatches targeted aliased functions (for example, one patch targets __do_sys_fork and another targets __x64_sys_fork), they were permitted since they resided in different patches. When an atomic replace patch is loaded, klp_add_nops() loops over all previously patched functions and adds nops for them to the new patch. Since nop deduplication is based on old_name, nops for both aliases are added. Later, this check in klp_init_object_loaded() resolves their addresses and finds duplicate old_func addresses. This causes the initialization to fail with -EINVAL, which prevents the system from atomically replacing existing livepatches and might force a reboot if the old patches cannot be safely disabled manually. > + } > + } > + > ret = kallsyms_lookup_size_offset((unsigned long)func->old_func, -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
