Messages in this thread | | | Date | Tue, 13 Mar 2018 15:55:36 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH v10 07/10] livepatch: Correctly handle atomic replace for not yet loaded modules |
| |
On Wed 2018-03-07 09:20:36, Petr Mladek wrote: > The atomic replace feature uses dynamically allocated struct klp_func to > handle functions that will no longer be patched. These structures are > of the type KLP_FUNC_NOP. They cause the ftrace handler to jump to > the original code. But the address of the original code is not known > until the patched module is loaded. > > This patch allows the late initialization. Also it adds a sanity check > into the ftrace handler. > > diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c > index 54b3c379bb0f..1f5c3eea9ee1 100644 > --- a/kernel/livepatch/patch.c > +++ b/kernel/livepatch/patch.c > @@ -118,7 +118,12 @@ static void notrace klp_ftrace_handler(unsigned long ip, > } > } > > + /* Survive ugly mistakes, for example, when handling NOPs. */ > + if (WARN_ON_ONCE(!func->new_func)) > + goto unlock;
JFYI, this is not enough. We really have to skip klp_arch_set_pc() for NOPs. Otherwise, we end up in an infinite loop. NOPs cause that we go back to the beginning of the original function, enter the ftrace handler again, ...
I am going to fix this in v11.
> + > klp_arch_set_pc(regs, (unsigned long)func->new_func); > + > unlock: > preempt_enable_notrace();
Kudos to Mirek for testing and hitting the problem.
Best Regards, Petr
| |