linux-kernel - Re: [4.4,06/28] rcu: Allow for page faults in NMI handlers

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1510240648.2465.46.camel@codethink.co.uk>
Date:   Thu, 09 Nov 2017 15:17:28 +0000
From:   Ben Hutchings <ben.hutchings@...ethink.co.uk>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     stable@...r.kernel.org,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [4.4,06/28] rcu: Allow for page faults in NMI handlers

On Mon, 2017-10-16 at 18:11 +0200, gregkh@...uxfoundation.org wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> 
> commit 28585a832602747cbfa88ad8934013177a3aae38 upstream.
> 
> A number of architecture invoke rcu_irq_enter() on exception entry in
> order to allow RCU read-side critical sections in the exception handler
> when the exception is from an idle or nohz_full CPU.  This works, at
> least unless the exception happens in an NMI handler.  In that case,
> rcu_nmi_enter() would already have exited the extended quiescent state,
> which would mean that rcu_irq_enter() would (incorrectly) cause RCU
> to think that it is again in an extended quiescent state.  This will
> in turn result in lockdep splats in response to later RCU read-side
> critical sections.
> 
> This commit therefore causes rcu_irq_enter() and rcu_irq_exit() to
> take no action if there is an rcu_nmi_enter() in effect, thus avoiding
> the unscheduled return to RCU quiescent state.  This in turn should
> make the kernel safe for on-demand RCU voyeurism.
> 
> Link: http://lkml.kernel.org/r/20170922211022.GA18084@linux.vnet.ibm.com
> 
> Cc: stable@...r.kernel.org
> Fixes: 0be964be0 ("module: Sanitize RCU usage and locking")
> > Reported-by: Steven Rostedt <rostedt@...dmis.org>
> > Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
>  kernel/rcu/tree.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -759,6 +759,12 @@ void rcu_irq_exit(void)
>  
>  	local_irq_save(flags);
>  	rdtp = this_cpu_ptr(&rcu_dynticks);
> +
> +	/* Page faults can happen in NMI handlers, so check... */
> +	if (READ_ONCE(rdtp->dynticks_nmi_nesting))
> +		return;

Shouldn't there be a local_irq_restore() on this return path?  Or does
this condition imply that IRQs were already disabled?

> +	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_exit() invoked with irqs enabled!!!");

I don't see why you added RCU_LOCKDEP_WARN() here.  Prior to 4.5 it's
not an error to call this function with IRQs disabled.  And after
calling local_irq_save(), it's redundant to assert that IRQs are
disabled.

>  	oldval = rdtp->dynticks_nesting;
>  	rdtp->dynticks_nesting--;
>  	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> @@ -887,6 +893,12 @@ void rcu_irq_enter(void)
>  
>  	local_irq_save(flags);
>  	rdtp = this_cpu_ptr(&rcu_dynticks);
> +
> +	/* Page faults can happen in NMI handlers, so check... */
> +	if (READ_ONCE(rdtp->dynticks_nmi_nesting))
> +		return;
> +
> +	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_enter() invoked with irqs enabled!!!");

Same problems here.

Ben.

>  	oldval = rdtp->dynticks_nesting;
>  	rdtp->dynticks_nesting++;
>  	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
-- 
Ben Hutchings
Software Developer, Codethink Ltd.