linux-kernel - Re: [RFC][PATCH 2/3] locking,entry: #PF vs TRACE

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 7 Aug 2020 16:21:48 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     tglx@...utronix.de, mingo@...nel.org, will@...nel.org,
        x86@...nel.org, linux-kernel@...r.kernel.org, elver@...gle.com,
        paulmck@...nel.org, rjw@...ysocki.net
Subject: Re: [RFC][PATCH 2/3] locking,entry: #PF vs TRACE_IRQFLAGS

On Fri, 07 Aug 2020 21:23:38 +0200
Peter Zijlstra <peterz@...radead.org> wrote:

> Much of the complexity in irqenter_{enter,exit}() is due to #PF being
> the sole exception that can schedule from kernel context.
> 
> One additional wrinkle with #PF is that it is non-maskable, it can
> happen _anywhere_. Due to this, and the wonders of tracing, we can get
> the 'normal' NMI nesting vs TRACE_IRQFLAGS:
> 
> 	local_irq_disable()
> 	  raw_local_irq_disable();
> 	  trace_hardirqs_off();
> 
> 	local_irq_enable();

Do you mean to have that ';' there? That is, it the below is called
from local_irq_enable(), right? A ';' means that local_irq_enable()
is completed.


> 	  trace_hardirqs_on();
> 	  <#PF>
> 	    trace_hardirqs_off()
> 	    ...
> 	    if (!regs_irqs_disabled(regs)

regs has it disabled, so this is false, right?

> 	      trace_hardirqs_on();
> 	  </#PF>

I missed the '/' in the above. At first I thought this was another page
fault :-/

> 	  // WHOOPS -- lockdep thinks IRQs are disabled again!
> 	  raw_local_irqs_enable();
> 
> Rework irqenter_{enter,exit}() to save/restore the software state.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  include/linux/entry-common.h |    1 
>  kernel/entry/common.c        |   52 ++++++++++++++++++++-----------------------
>  2 files changed, 26 insertions(+), 27 deletions(-)
> 
> --- a/include/linux/entry-common.h
> +++ b/include/linux/entry-common.h
> @@ -310,6 +310,7 @@ void irqentry_exit_to_user_mode(struct p
>  #ifndef irqentry_state
>  typedef struct irqentry_state {
>  	bool	exit_rcu;
> +	bool	irqs_enabled;

Instead of passing a structure around, should we look at converting
"irqentry_state" into a flags field?

-- Steve


>  } irqentry_state_t;
>  #endif
>