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.

Cc: sta...@vger.kernel.org
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---

Hi Ingo-

You asked me to look for an irqflag tracing bug, so I found one :)

 arch/x86/entry/entry_64.S | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index a2b30ec69497..3c288f260fdf 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -51,15 +51,19 @@ ENTRY(native_usergs_sysret64)
 END(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
@@ -943,11 +947,13 @@ ENTRY(native_load_gs_index)
        FRAME_BEGIN
        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
        FRAME_END
        ret
-- 
2.13.6

Reply via email to