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

[Petr Mladek <pmladek@suse.com>]

On Thu 2026-10-01 12:32:28, Harry Hsu wrote:
> On Fri, 18 Sep 2026, Miroslav Benes wrote:
> 
> > Could we also check prev_func/func->nop in the check in
> > klp_init_object_loaded() and have a pass if true?
> 
> Thanks for the suggestion.
> 
> Yes, I've updated the check to skip the pair when both klp_funcs are
> nops. Two nops added by klp_add_nops() for replaced aliases are
> interchangeable, so an atomic replace patch that inherits nops for
> __do_sys_fork and __x64_sys_fork from two previous patches now loads
> fine. A non-nop func colliding with a nop (or another non-nop func)
> is still rejected, since the nop would land on top of the real
> replacement on ops->func_stack and silently disable it.
> 
> The incremental change on top of v4 1/5 is:
> 
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -894,6 +894,8 @@ static int klp_init_object_loaded(struct klp_patch *patch,
>  			if (prev_func == func)
>  				break;
>  			if (prev_func->old_func == func->old_func) {
> +				if (prev_func->nop && func->nop)
> +					continue;
>  				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;
> 
> If this direction looks good, I'll resend the series. Petr, should I
> just bundle everything into a v5, or do you have changes on your side
> that I should wait for?

I am afraid that this is still not enough.

Problem:

It would work when the livepatch gets applied the nops are removed.

But it might cause problems when the transition gets reverted
in the middle. All processes should go back to use the previous
implementations (original code or previous livepatch).
But the ftrace handler would just go back from one nop to the other.


Solutions:

1. A solution would be to create only one not in the first place.

2. It seems that we are close to the scoped atomic replace feature.
   It will prevent loading conflicting livepatches in parallel.
   Well, it currently checks only the function names. We should
   add another another check which would detect the conflict
   also by the alias.


Best Regards,
Petr

PS: I am sorry for the late reply. This patchset is far from trivial
    because it handles several levels of "what will happen if"
    and makes my head spin. And it has lower priority because
    it handles corner cases.

    Also I always forget all the details before I get an answer.
    It seems that we are all in the same trap. ;-)