lkml.org 
[lkml]   [2018]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2018-03-13 15:56    [W:4.053 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site