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

[Miroslav Benes]

On Fri, 23 Oct 2026, Petr Mladek wrote:

> 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.

Hm, you are right. There will be two nop functions on the stack in this 
case and the redirection in the handler would not work.

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

Not a bad idea.

> 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.

-ETOOMANYDEPENDENCIESBETWEENPATCHSETS. If the first solution is not 
difficult to implement, it might be better.

> 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. ;-)

Yes, it took me a while to page it all in again :).

Miroslav