linux-kernel - Re: [tip:x86/asm] x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion

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]

Date:	Sat, 18 Jul 2015 15:23:57 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Andrew Lutomirski <luto@...nel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Kees Cook <keescook@...omium.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Oleg Nesterov <oleg@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Denys Vlasenko <vda.linux@...glemail.com>,
	Rik van Riel <riel@...hat.com>,
	Borislav Petkov <bp@...en8.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Brian Gerst <brgerst@...il.com>,
	"linux-tip-commits@...r.kernel.org" 
	<linux-tip-commits@...r.kernel.org>
Subject: Re: [tip:x86/asm] x86/irq, context_tracking: Document how IRQ
 context tracking works and add an RCU assertion

On Tue, Jul 14, 2015 at 04:33:39PM -0700, Andy Lutomirski wrote:
> On Tue, Jul 14, 2015 at 4:26 PM, Frederic Weisbecker <fweisbec@...il.com> wrote:
> > On Tue, Jul 07, 2015 at 03:54:32AM -0700, tip-bot for Andy Lutomirski wrote:
> >> Commit-ID:  0333a209cbf600e980fc55c24878a56f25f48b65
> >> Gitweb:     http://git.kernel.org/tip/0333a209cbf600e980fc55c24878a56f25f48b65
> >> Author:     Andy Lutomirski <luto@...nel.org>
> >> AuthorDate: Fri, 3 Jul 2015 12:44:34 -0700
> >> Committer:  Ingo Molnar <mingo@...nel.org>
> >> CommitDate: Tue, 7 Jul 2015 10:59:10 +0200
> >>
> >> x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion
> >>
> >> Signed-off-by: Andy Lutomirski <luto@...nel.org>
> >> Cc: Andy Lutomirski <luto@...capital.net>
> >> Cc: Borislav Petkov <bp@...en8.de>
> >> Cc: Brian Gerst <brgerst@...il.com>
> >> Cc: Denys Vlasenko <dvlasenk@...hat.com>
> >> Cc: Denys Vlasenko <vda.linux@...glemail.com>
> >> Cc: Frederic Weisbecker <fweisbec@...il.com>
> >> Cc: H. Peter Anvin <hpa@...or.com>
> >> Cc: Kees Cook <keescook@...omium.org>
> >> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> >> Cc: Oleg Nesterov <oleg@...hat.com>
> >> Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> >> Cc: Peter Zijlstra <peterz@...radead.org>
> >> Cc: Rik van Riel <riel@...hat.com>
> >> Cc: Thomas Gleixner <tglx@...utronix.de>
> >> Cc: paulmck@...ux.vnet.ibm.com
> >> Link: http://lkml.kernel.org/r/e8bdc4ed0193fb2fd130f3d6b7b8023e2ec1ab62.1435952415.git.luto@kernel.org
> >> Signed-off-by: Ingo Molnar <mingo@...nel.org>
> >> ---
> >>  arch/x86/kernel/irq.c | 15 +++++++++++++++
> >>  1 file changed, 15 insertions(+)
> >>
> >> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> >> index 88b36648..6233de0 100644
> >> --- a/arch/x86/kernel/irq.c
> >> +++ b/arch/x86/kernel/irq.c
> >> @@ -216,8 +216,23 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
> >>       unsigned vector = ~regs->orig_ax;
> >>       unsigned irq;
> >>
> >> +     /*
> >> +      * NB: Unlike exception entries, IRQ entries do not reliably
> >> +      * handle context tracking in the low-level entry code.  This is
> >> +      * because syscall entries execute briefly with IRQs on before
> >> +      * updating context tracking state, so we can take an IRQ from
> >> +      * kernel mode with CONTEXT_USER.  The low-level entry code only
> >> +      * updates the context if we came from user mode, so we won't
> >> +      * switch to CONTEXT_KERNEL.  We'll fix that once the syscall
> >> +      * code is cleaned up enough that we can cleanly defer enabling
> >> +      * IRQs.
> >> +      */
> >> +
> >
> > Now is it a problem to take interrupts in kernel mode with CONTEXT_USER?
> > I'm not sure it's worth trying to make it not happen.
> 
> It's not currently a problem, but it would be nice if we could do the
> equivalent of:
> 
> if (user_mode(regs)) {
>   user_exit();  (or enter_from_user_mode or whatever)
> } else {
>   // don't bother -- already in CONTEXT_KERNEL
> }

This was the initial implementation of context tracking but it was terribly
buggy. What if we enter the kernel, we haven't yet got a change to call
context_tracking_user_exit() and we get an exception in the kernel entry
path? user_mode(regs) will return the wrong value and bad things happen.

This is why context tracking needs its own tracking state, because we are always
out of sync with the real processor context anyway.

> 
> i.e. the same thing that do_general_protection, etc do in -tip.  That
> would get rid of any need to store the previous context.
> 
> Currently we can't because of syscalls and maybe because of KVM.  KVM
> has a weird fake interrupt thing.
> 
> >
> >>       entering_irq();
> >>
> >> +     /* entering_irq() tells RCU that we're not quiescent.  Check it. */
> >> +     rcu_lockdep_assert(rcu_is_watching(), "IRQ failed to wake up RCU");
> >
> > Why do we need to check that?
> 
> Sanity check.  If we're changing a bunch of context tracking details,
> I want to assert that it actually works.

But we call rcu_irq_enter() right before.

It's more or less like doing:

local_irq_disable();
WARN_ON(!irqs_disabled());
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/