Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Ingo Molnar
* Josh Poimboeuf wrote: > Dave was right, my patch was obviously bogus. I couldn't figure out a > real reproducer, so I made an artificial one (see below) and can confirm > that your patch fixes it. > > I would resubmit the patch, but now you're the author, so I'm not

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Ingo Molnar
* Josh Poimboeuf wrote: > Dave was right, my patch was obviously bogus. I couldn't figure out a > real reproducer, so I made an artificial one (see below) and can confirm > that your patch fixes it. > > I would resubmit the patch, but now you're the author, so I'm not sure > how that works

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Andy Lutomirski
On Wed, Feb 14, 2018 at 7:39 AM, Ingo Molnar wrote: > > * Dave Hansen wrote: > >> On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: >> > --- a/arch/x86/entry/entry_64.S >> > +++ b/arch/x86/entry/entry_64.S >> > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) >>

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Andy Lutomirski
On Wed, Feb 14, 2018 at 7:39 AM, Ingo Molnar wrote: > > * Dave Hansen wrote: > >> On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: >> > --- a/arch/x86/entry/entry_64.S >> > +++ b/arch/x86/entry/entry_64.S >> > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) >> > UNWIND_HINT_REGS >> >

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Thomas Gleixner
On Wed, 14 Feb 2018, Ingo Molnar wrote: > * Dave Hansen wrote: > > > On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > > > --- a/arch/x86/entry/entry_64.S > > > +++ b/arch/x86/entry/entry_64.S > > > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > > > UNWIND_HINT_REGS > > >

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Thomas Gleixner
On Wed, 14 Feb 2018, Ingo Molnar wrote: > * Dave Hansen wrote: > > > On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > > > --- a/arch/x86/entry/entry_64.S > > > +++ b/arch/x86/entry/entry_64.S > > > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > > > UNWIND_HINT_REGS > > >

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Josh Poimboeuf
On Wed, Feb 14, 2018 at 08:39:11AM +0100, Ingo Molnar wrote: > > * Dave Hansen wrote: > > > On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > > > --- a/arch/x86/entry/entry_64.S > > > +++ b/arch/x86/entry/entry_64.S > > > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > > >

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-14 Thread Josh Poimboeuf
On Wed, Feb 14, 2018 at 08:39:11AM +0100, Ingo Molnar wrote: > > * Dave Hansen wrote: > > > On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > > > --- a/arch/x86/entry/entry_64.S > > > +++ b/arch/x86/entry/entry_64.S > > > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > > > UNWIND_HINT_REGS > > >

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Ingo Molnar
* Dave Hansen wrote: > On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > > --- a/arch/x86/entry/entry_64.S > > +++ b/arch/x86/entry/entry_64.S > > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > > UNWIND_HINT_REGS > > DISABLE_INTERRUPTS(CLBR_ANY) > >

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Ingo Molnar
* Dave Hansen wrote: > On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > > --- a/arch/x86/entry/entry_64.S > > +++ b/arch/x86/entry/entry_64.S > > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > > UNWIND_HINT_REGS > > DISABLE_INTERRUPTS(CLBR_ANY) > > TRACE_IRQS_OFF_DEBUG > > +

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Ingo Molnar
* Josh Poimboeuf wrote: > I haven't actually seen any real-world bugs caused by this, so I'm not > sure how theoretical it is. I just stumbled upon it in code review when > looking for another bug. I believe it's a real bug, but the fix is wrong with irq tracing or

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Ingo Molnar
* Josh Poimboeuf wrote: > I haven't actually seen any real-world bugs caused by this, so I'm not > sure how theoretical it is. I just stumbled upon it in code review when > looking for another bug. I believe it's a real bug, but the fix is wrong with irq tracing or lockdep enabled as Dave

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Dave Hansen
On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > UNWIND_HINT_REGS > DISABLE_INTERRUPTS(CLBR_ANY) > TRACE_IRQS_OFF_DEBUG > + RESTORE_CR3 scratch_reg=%r15

Re: [PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Dave Hansen
On 02/13/2018 06:27 PM, Josh Poimboeuf wrote: > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -1167,10 +1167,10 @@ ENTRY(paranoid_exit) > UNWIND_HINT_REGS > DISABLE_INTERRUPTS(CLBR_ANY) > TRACE_IRQS_OFF_DEBUG > + RESTORE_CR3 scratch_reg=%r15

[PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Josh Poimboeuf
The paranoid exit code only restores the saved CR3 when it switches back to the user GS. However, even in the kernel GS case, it's possible that it needs to restore a user CR3, if for example, the paranoid exception occurred in the syscall exit path between SWITCH_TO_USER_CR3_STACK and SWAPGS.

[PATCH] x86/entry/64: Fix CR3 restore order in paranoid_exit()

2018-02-13 Thread Josh Poimboeuf
The paranoid exit code only restores the saved CR3 when it switches back to the user GS. However, even in the kernel GS case, it's possible that it needs to restore a user CR3, if for example, the paranoid exception occurred in the syscall exit path between SWITCH_TO_USER_CR3_STACK and SWAPGS.