linux-kernel - Re: [RFC][PATCH 6/7] context_tracking: Provide SMP ordering using RCU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YVHWJi6AQwyH2Eys@alley>
Date:   Mon, 27 Sep 2021 16:33:10 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Joel Savitz <jsavitz@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>, gor@...ux.ibm.com,
        jpoimboe@...hat.com, jikos@...nel.org, mbenes@...e.cz,
        mingo@...nel.org, linux-kernel@...r.kernel.org,
        joe.lawrence@...hat.com, fweisbec@...il.com, tglx@...utronix.de,
        hca@...ux.ibm.com, svens@...ux.ibm.com, sumanthk@...ux.ibm.com,
        live-patching@...r.kernel.org, paulmck@...nel.org
Subject: Re: [RFC][PATCH 6/7] context_tracking: Provide SMP ordering using RCU

On Fri 2021-09-24 14:57:05, Joel Savitz wrote:
> On Wed, Sep 22, 2021 at 01:05:12PM +0200, Peter Zijlstra wrote:
> > ---
> >  include/linux/context_tracking_state.h |   12 ++++++++++++
> >  kernel/context_tracking.c              |    7 ++++---
> >  2 files changed, 16 insertions(+), 3 deletions(-)
> > 
> > --- a/include/linux/context_tracking_state.h
> > +++ b/include/linux/context_tracking_state.h
> > @@ -45,11 +45,23 @@ static __always_inline bool context_trac
> >  {
> >  	return __this_cpu_read(context_tracking.state) == CONTEXT_USER;
> >  }
> > +
> > +static __always_inline bool context_tracking_state_cpu(int cpu)
> > +{
> > +	struct context_tracking *ct = per_cpu_ptr(&context_tracking);
> > +
> > +	if (!context_tracking_enabled() || !ct->active)
> > +		return CONTEXT_DISABLED;
> > +
> > +	return ct->state;
> > +}
> > +
> >  #else
> >  static inline bool context_tracking_in_user(void) { return false; }
> >  static inline bool context_tracking_enabled(void) { return false; }
> >  static inline bool context_tracking_enabled_cpu(int cpu) { return false; }
> >  static inline bool context_tracking_enabled_this_cpu(void) { return false; }
> > +static inline bool context_tracking_state_cpu(int cpu) { return CONTEXT_DISABLED; }
> >  #endif /* CONFIG_CONTEXT_TRACKING */
> >  
> >  #endif
> 
> Should context_tracking_state_cpu return an enum ctx_state rather than a
> bool? It appears to be doing an implicit cast.

Great catch!

> I don't know if it possible to run livepatch with
> CONFIG_CONTEXT_TRACKING disabled,

It should work with CONFIG_CONTEXT_TRACKING. The original code
migrates the task only when it is not running on any CPU and patched
functions are not on the stack. The stack check covers also
the user context.

> modified by patch 7 will always consider the transition complete even if
> the current task is in kernel mode. Also in the general case, the CPU
> will consider the task complete if has ctx_state CONTEXT_GUEST though the
> condition does not make it explicit.

Yup, we should avoid the enum -> bool cast, definitely.

Best Regards,
Petr