Skip to content

Commit ca37e57

Browse files
amlutoIngo Molnar
authored andcommitted
x86/entry/64: Add missing irqflags tracing to native_load_gs_index()
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 <[email protected]> Cc: Borislav Petkov <[email protected]> Cc: Brian Gerst <[email protected]> Cc: Dave Hansen <[email protected]> Cc: Josh Poimboeuf <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Thomas Gleixner <[email protected]> Cc: [email protected] Link: http://lkml.kernel.org/r/e1aeb0e6ba8dd430ec36c8a35e63b429698b4132.1511411918.git.luto@kernel.org Signed-off-by: Ingo Molnar <[email protected]>
1 parent f68d62a commit ca37e57

File tree

1 file changed

+8
-2
lines changed

1 file changed

+8
-2
lines changed

arch/x86/entry/entry_64.S

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,19 @@ ENTRY(native_usergs_sysret64)
5151
END(native_usergs_sysret64)
5252
#endif /* CONFIG_PARAVIRT */
5353

54-
.macro TRACE_IRQS_IRETQ
54+
.macro TRACE_IRQS_FLAGS flags:req
5555
#ifdef CONFIG_TRACE_IRQFLAGS
56-
bt $9, EFLAGS(%rsp) /* interrupts off? */
56+
bt $9, \flags /* interrupts off? */
5757
jnc 1f
5858
TRACE_IRQS_ON
5959
1:
6060
#endif
6161
.endm
6262

63+
.macro TRACE_IRQS_IRETQ
64+
TRACE_IRQS_FLAGS EFLAGS(%rsp)
65+
.endm
66+
6367
/*
6468
* When dynamic function tracer is enabled it will add a breakpoint
6569
* to all locations that it is about to modify, sync CPUs, update
@@ -943,11 +947,13 @@ ENTRY(native_load_gs_index)
943947
FRAME_BEGIN
944948
pushfq
945949
DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
950+
TRACE_IRQS_OFF
946951
SWAPGS
947952
.Lgs_change:
948953
movl %edi, %gs
949954
2: ALTERNATIVE "", "mfence", X86_BUG_SWAPGS_FENCE
950955
SWAPGS
956+
TRACE_IRQS_FLAGS (%rsp)
951957
popfq
952958
FRAME_END
953959
ret

0 commit comments

Comments
 (0)