Re: [PATCH v4 1/5] livepatch: Fail object initialization on duplicate patched function

[sashiko-bot]

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 <x90613@xxxxxxxxx>

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/20260908120325.299649-1-pmladek@xxxxxxxx?part=1