Re: [RFC PATCH v1.9 14/14] livepatch: update task universe when exiting kernel

[Josh Poimboeuf]

On Thu, Apr 14, 2016 at 10:47:04AM +0200, Miroslav Benes wrote:
> On Fri, 25 Mar 2016, Josh Poimboeuf wrote:
> 
> > Update a tasks's universe when returning from a system call or user
> > space interrupt, or after handling a signal.
> > 
> > This greatly increases the chances of a patch operation succeeding.  If
> > a task is I/O bound, it can switch universes when returning from a
> > system call.  If a task is CPU bound, it can switch universes when
> > returning from an interrupt.  If a task is sleeping on a to-be-patched
> > function, the user can send SIGSTOP and SIGCONT to force it to switch.
> > 
> > Since the idle "swapper" tasks don't ever exit the kernel, they're
> > updated from within the idle loop.
> 
> Well, I am still not familiarized enough with Andy's recent rework of 
> entry stuff, but I think all of this is correct. Maybe I would add 
> a note to the changelog, that since TIF_KLP_NEED_UPDATE is defined 14th 
> bit it is also automatically included in _TIF_ALLWORK_MASKS. 

To be honest, putting it in the range of _TIF_ALLWORK_MASK was an
accident.  I think the comments in thread_info.h need to be improved a
bit to make that clearer.

> > Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > ---
> >  arch/x86/entry/common.c            |  6 +++++-
> >  arch/x86/include/asm/thread_info.h |  2 ++
> >  include/linux/livepatch.h          |  2 ++
> >  kernel/livepatch/transition.c      | 37 +++++++++++++++++++++++++++++++++----
> >  kernel/sched/idle.c                |  4 ++++
> >  5 files changed, 46 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index e79d93d..94639dd 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/context_tracking.h>
> >  #include <linux/user-return-notifier.h>
> >  #include <linux/uprobes.h>
> > +#include <linux/livepatch.h>
> >  
> >  #include <asm/desc.h>
> >  #include <asm/traps.h>
> > @@ -202,7 +203,7 @@ long syscall_trace_enter(struct pt_regs *regs)
> >  
> >  #define EXIT_TO_USERMODE_LOOP_FLAGS				\
> >  	(_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE |	\
> > -	 _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY)
> > +	 _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY | _TIF_KLP_NEED_UPDATE)
> >  
> >  static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
> >  {
> > @@ -236,6 +237,9 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
> >  		if (cached_flags & _TIF_USER_RETURN_NOTIFY)
> >  			fire_user_return_notifiers();
> >  
> > +		if (unlikely(cached_flags & _TIF_KLP_NEED_UPDATE))
> > +			klp_update_task_universe(current);
> > +
> 
> There is a comment at the beginning of this function which should be 
> updated as well I think.

Yeah, agreed.

-- 
Josh