[Linux-stable-mirror] Patch "x86/entry/64: Add missing irqflags tracing to native_load_gs_index()" has been added to the 4.9-stable tree
This is a note to let you know that I've just added the patch titled
x86/entry/64: Add missing irqflags tracing to native_load_gs_index()
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: x86-entry-64-add-missing-irqflags-tracing-to-native_load_gs_index.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From ca37e57bbe0cf1455ea3e84eb89ed04a132d59e1 Mon Sep 17 00:00:00 2001
From: Andy Lutomirski luto@kernel.org Date: Wed, 22 Nov 2017 20:39:16 -0800 Subject: x86/entry/64: Add missing irqflags tracing to native_load_gs_index()
From: Andy Lutomirski luto@kernel.org
commit ca37e57bbe0cf1455ea3e84eb89ed04a132d59e1 upstream.
Running this code with IRQs enabled (where dummy_lock is a spinlock):
static void check_load_gs_index(void) { /* This will fail. */ load_gs_index(0xffff);
spin_lock(&dummy_lock); spin_unlock(&dummy_lock); }
Will generate a lockdep warning. The issue is that the actual write to %gs would cause an exception with IRQs disabled, and the exception handler would, as an inadvertent side effect, update irqflag tracing to reflect the IRQs-off status. native_load_gs_index() would then turn IRQs back on and return with irqflag tracing still thinking that IRQs were off. The dummy lock-and-unlock causes lockdep to notice the error and warn.
Fix it by adding the missing tracing.
Apparently nothing did this in a context where it mattered. I haven't tried to find a code path that would actually exhibit the warning if appropriately nasty user code were running.
I suspect that the security impact of this bug is very, very low -- production systems don't run with lockdep enabled, and the warning is mostly harmless anyway.
Found during a quick audit of the entry code to try to track down an unrelated bug that Ingo found in some still-in-development code.
Signed-off-by: Andy Lutomirski luto@kernel.org Cc: Borislav Petkov bpetkov@suse.de Cc: Brian Gerst brgerst@gmail.com Cc: Dave Hansen dave.hansen@intel.com Cc: Josh Poimboeuf jpoimboe@redhat.com Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Link: http://lkml.kernel.org/r/e1aeb0e6ba8dd430ec36c8a35e63b429698b4132.1511411918... Signed-off-by: Ingo Molnar mingo@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- arch/x86/entry/entry_64.S | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
--- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -54,15 +54,19 @@ ENTRY(native_usergs_sysret64) ENDPROC(native_usergs_sysret64) #endif /* CONFIG_PARAVIRT */
-.macro TRACE_IRQS_IRETQ +.macro TRACE_IRQS_FLAGS flags:req #ifdef CONFIG_TRACE_IRQFLAGS - bt $9, EFLAGS(%rsp) /* interrupts off? */ + bt $9, \flags /* interrupts off? */ jnc 1f TRACE_IRQS_ON 1: #endif .endm
+.macro TRACE_IRQS_IRETQ + TRACE_IRQS_FLAGS EFLAGS(%rsp) +.endm + /* * When dynamic function tracer is enabled it will add a breakpoint * to all locations that it is about to modify, sync CPUs, update @@ -868,11 +872,13 @@ idtentry simd_coprocessor_error do_simd ENTRY(native_load_gs_index) pushfq DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI) + TRACE_IRQS_OFF SWAPGS .Lgs_change: movl %edi, %gs 2: ALTERNATIVE "", "mfence", X86_BUG_SWAPGS_FENCE SWAPGS + TRACE_IRQS_FLAGS (%rsp) popfq ret END(native_load_gs_index)
Patches currently in stable-queue which might be from luto@kernel.org are
queue-4.9/x86-entry-64-add-missing-irqflags-tracing-to-native_load_gs_index.patch
If this hasn't been released yet, please hold off. It's buggy.
On Mon, Nov 27, 2017 at 4:41 AM, gregkh@linuxfoundation.org wrote:
This is a note to let you know that I've just added the patch titled
x86/entry/64: Add missing irqflags tracing to native_load_gs_index()to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: x86-entry-64-add-missing-irqflags-tracing-to-native_load_gs_index.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From ca37e57bbe0cf1455ea3e84eb89ed04a132d59e1 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski luto@kernel.org Date: Wed, 22 Nov 2017 20:39:16 -0800 Subject: x86/entry/64: Add missing irqflags tracing to native_load_gs_index()
From: Andy Lutomirski luto@kernel.org
commit ca37e57bbe0cf1455ea3e84eb89ed04a132d59e1 upstream.
Running this code with IRQs enabled (where dummy_lock is a spinlock):
static void check_load_gs_index(void) { /* This will fail. */ load_gs_index(0xffff);
spin_lock(&dummy_lock); spin_unlock(&dummy_lock);}
Will generate a lockdep warning. The issue is that the actual write to %gs would cause an exception with IRQs disabled, and the exception handler would, as an inadvertent side effect, update irqflag tracing to reflect the IRQs-off status. native_load_gs_index() would then turn IRQs back on and return with irqflag tracing still thinking that IRQs were off. The dummy lock-and-unlock causes lockdep to notice the error and warn.
Fix it by adding the missing tracing.
Apparently nothing did this in a context where it mattered. I haven't tried to find a code path that would actually exhibit the warning if appropriately nasty user code were running.
I suspect that the security impact of this bug is very, very low -- production systems don't run with lockdep enabled, and the warning is mostly harmless anyway.
Found during a quick audit of the entry code to try to track down an unrelated bug that Ingo found in some still-in-development code.
Signed-off-by: Andy Lutomirski luto@kernel.org Cc: Borislav Petkov bpetkov@suse.de Cc: Brian Gerst brgerst@gmail.com Cc: Dave Hansen dave.hansen@intel.com Cc: Josh Poimboeuf jpoimboe@redhat.com Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Link: http://lkml.kernel.org/r/e1aeb0e6ba8dd430ec36c8a35e63b429698b4132.1511411918... Signed-off-by: Ingo Molnar mingo@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
arch/x86/entry/entry_64.S | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
--- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -54,15 +54,19 @@ ENTRY(native_usergs_sysret64) ENDPROC(native_usergs_sysret64) #endif /* CONFIG_PARAVIRT */
-.macro TRACE_IRQS_IRETQ +.macro TRACE_IRQS_FLAGS flags:req #ifdef CONFIG_TRACE_IRQFLAGS
bt $9, EFLAGS(%rsp) /* interrupts off? */
bt $9, \flags /* interrupts off? */ jnc 1f TRACE_IRQS_ON1: #endif .endm
+.macro TRACE_IRQS_IRETQ
TRACE_IRQS_FLAGS EFLAGS(%rsp)+.endm
/*
- When dynamic function tracer is enabled it will add a breakpoint
- to all locations that it is about to modify, sync CPUs, update
@@ -868,11 +872,13 @@ idtentry simd_coprocessor_error do_simd ENTRY(native_load_gs_index) pushfq DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
TRACE_IRQS_OFF SWAPGS.Lgs_change: movl %edi, %gs 2: ALTERNATIVE "", "mfence", X86_BUG_SWAPGS_FENCE SWAPGS
TRACE_IRQS_FLAGS (%rsp) popfq retEND(native_load_gs_index)
Patches currently in stable-queue which might be from luto@kernel.org are
queue-4.9/x86-entry-64-add-missing-irqflags-tracing-to-native_load_gs_index.patch
On Wed, Nov 29, 2017 at 08:23:10AM -0800, Andy Lutomirski wrote:
If this hasn't been released yet, please hold off. It's buggy.
Ah crap, I missed this and just released it. Is there a fix for this in Linus's tree? How "buggy" is it?
thanks,
greg k-h
On Nov 30, 2017, at 12:44 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Nov 29, 2017 at 08:23:10AM -0800, Andy Lutomirski wrote: If this hasn't been released yet, please hold off. It's buggy.
Ah crap, I missed this and just released it. Is there a fix for this in Linus's tree? How "buggy" is it?
It breaks suspend/resume on some configuration that I haven't quite figured out. I think it only happens if (lockdep is on and some other unknown condition occurs) or (ftrace is actively tracing irq flags). I have a fix, but I was waiting to for the OP to test it.
I'll send the fix Linusward later today without the Tested-by if I don't hear anything.
thanks,
greg k-h
On Thu, Nov 30, 2017 at 6:29 AM, Andy Lutomirski luto@amacapital.net wrote:
On Nov 30, 2017, at 12:44 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Nov 29, 2017 at 08:23:10AM -0800, Andy Lutomirski wrote: If this hasn't been released yet, please hold off. It's buggy.
Ah crap, I missed this and just released it. Is there a fix for this in Linus's tree? How "buggy" is it?
It breaks suspend/resume on some configuration that I haven't quite figured out. I think it only happens if (lockdep is on and some other unknown condition occurs) or (ftrace is actively tracing irq flags). I have a fix, but I was waiting to for the OP to test it.
I'll send the fix Linusward later today without the Tested-by if I don't hear anything.
I think the thing to do is to revert the patch from -stable. The bug it fixes is very minor, and the regression is that it made a pre-existing bug in some nearly-undebuggable core resume code much easier to hit. I don't feel comfortable with a backport of the latter fix until it has a good long soak in Linus' tree.
On Thu, Nov 30, 2017 at 10:50:22AM -0800, Andy Lutomirski wrote:
On Thu, Nov 30, 2017 at 6:29 AM, Andy Lutomirski luto@amacapital.net wrote:
On Nov 30, 2017, at 12:44 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Nov 29, 2017 at 08:23:10AM -0800, Andy Lutomirski wrote: If this hasn't been released yet, please hold off. It's buggy.
Ah crap, I missed this and just released it. Is there a fix for this in Linus's tree? How "buggy" is it?
It breaks suspend/resume on some configuration that I haven't quite figured out. I think it only happens if (lockdep is on and some other unknown condition occurs) or (ftrace is actively tracing irq flags). I have a fix, but I was waiting to for the OP to test it.
I'll send the fix Linusward later today without the Tested-by if I don't hear anything.
I think the thing to do is to revert the patch from -stable. The bug it fixes is very minor, and the regression is that it made a pre-existing bug in some nearly-undebuggable core resume code much easier to hit. I don't feel comfortable with a backport of the latter fix until it has a good long soak in Linus' tree.
Ok, now reverted. Should I do the same thing for the patch in 4.14-stable as well?
thanks,
greg k-h
On Dec 4, 2017, at 4:02 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Nov 30, 2017 at 10:50:22AM -0800, Andy Lutomirski wrote:
On Thu, Nov 30, 2017 at 6:29 AM, Andy Lutomirski luto@amacapital.net wrote:
On Nov 30, 2017, at 12:44 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Nov 29, 2017 at 08:23:10AM -0800, Andy Lutomirski wrote: If this hasn't been released yet, please hold off. It's buggy.
Ah crap, I missed this and just released it. Is there a fix for this in Linus's tree? How "buggy" is it?
It breaks suspend/resume on some configuration that I haven't quite figured out. I think it only happens if (lockdep is on and some other unknown condition occurs) or (ftrace is actively tracing irq flags). I have a fix, but I was waiting to for the OP to test it.
I'll send the fix Linusward later today without the Tested-by if I don't hear anything.
I think the thing to do is to revert the patch from -stable. The bug it fixes is very minor, and the regression is that it made a pre-existing bug in some nearly-undebuggable core resume code much easier to hit. I don't feel comfortable with a backport of the latter fix until it has a good long soak in Linus' tree.
Ok, now reverted. Should I do the same thing for the patch in 4.14-stable as well?
Yes please.
thanks,
greg k-h
On Mon, Dec 04, 2017 at 05:52:55AM -0800, Andy Lutomirski wrote:
On Dec 4, 2017, at 4:02 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Nov 30, 2017 at 10:50:22AM -0800, Andy Lutomirski wrote:
On Thu, Nov 30, 2017 at 6:29 AM, Andy Lutomirski luto@amacapital.net wrote:
On Nov 30, 2017, at 12:44 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Nov 29, 2017 at 08:23:10AM -0800, Andy Lutomirski wrote: If this hasn't been released yet, please hold off. It's buggy.
Ah crap, I missed this and just released it. Is there a fix for this in Linus's tree? How "buggy" is it?
It breaks suspend/resume on some configuration that I haven't quite figured out. I think it only happens if (lockdep is on and some other unknown condition occurs) or (ftrace is actively tracing irq flags). I have a fix, but I was waiting to for the OP to test it.
I'll send the fix Linusward later today without the Tested-by if I don't hear anything.
I think the thing to do is to revert the patch from -stable. The bug it fixes is very minor, and the regression is that it made a pre-existing bug in some nearly-undebuggable core resume code much easier to hit. I don't feel comfortable with a backport of the latter fix until it has a good long soak in Linus' tree.
Ok, now reverted. Should I do the same thing for the patch in 4.14-stable as well?
Yes please.
Great, now done.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Andy Lutomirski -
Andy Lutomirski -
Greg KH -
gregkh＠linuxfoundation.org