x86/asm/irq: Stop relying on magic JMP behavior for early_idt_handlers

commit 425be5679fd292a3c36cb1fe423086708a99f11a upstream.

The early_idt_handlers asm code generates an array of entry
points spaced nine bytes apart.  It's not really clear from that
code or from the places that reference it what's going on, and
the code only works in the first place because GAS never
generates two-byte JMP instructions when jumping to global
labels.

Clean up the code to generate the correct array stride (member size)
explicitly. This should be considerably more robust against
screw-ups, as GAS will warn if a .fill directive has a negative
count.  Using '. =' to advance would have been even more robust
(it would generate an actual error if it tried to move
backwards), but it would pad with nulls, confusing anyone who
tries to disassemble the code.  The new scheme should be much
clearer to future readers.

While we're at it, improve the comments and rename the array and
common code.

Binutils may start relaxing jumps to non-weak labels.  If so,
this change will fix our build, and we may need to backport this
change.

Before, on x86_64:

  0000000000000000 <early_idt_handlers>:
     0:   6a 00                   pushq  $0x0
     2:   6a 00                   pushq  $0x0
     4:   e9 00 00 00 00          jmpq   9 <early_idt_handlers+0x9>
                          5: R_X86_64_PC32        early_idt_handler-0x4
  ...
    48:   66 90                   xchg   %ax,%ax
    4a:   6a 08                   pushq  $0x8
    4c:   e9 00 00 00 00          jmpq   51 <early_idt_handlers+0x51>
                          4d: R_X86_64_PC32       early_idt_handler-0x4
  ...
   117:   6a 00                   pushq  $0x0
   119:   6a 1f                   pushq  $0x1f
   11b:   e9 00 00 00 00          jmpq   120 <early_idt_handler>
                          11c: R_X86_64_PC32      early_idt_handler-0x4

After:

  0000000000000000 <early_idt_handler_array>:
     0:   6a 00                   pushq  $0x0
     2:   6a 00                   pushq  $0x0
     4:   e9 14 01 00 00          jmpq   11d <early_idt_handler_common>
  ...
    48:   6a 08                   pushq  $0x8
    4a:   e9 d1 00 00 00          jmpq   120 <early_idt_handler_common>
    4f:   cc                      int3
    50:   cc                      int3
  ...
   117:   6a 00                   pushq  $0x0
   119:   6a 1f                   pushq  $0x1f
   11b:   eb 03                   jmp    120 <early_idt_handler_common>
   11d:   cc                      int3
   11e:   cc                      int3
   11f:   cc                      int3

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: H. Peter Anvin <hpa@linux.intel.com>
Cc: Binutils <binutils@sourceware.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: H.J. Lu <hjl.tools@gmail.com>
Cc: Jan Beulich <JBeulich@suse.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/ac027962af343b0c599cbfcf50b945ad2ef3d7a8.1432336324.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index c48a95035a77888c9ba2210f2f33fd19250a952b..4dde707a6ff73e961b6fd0f95a13a2ec5c53f434 100644 (file)
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -212,8 +212,19 @@
 #define TLS_SIZE (GDT_ENTRY_TLS_ENTRIES * 8)
 
 #ifdef __KERNEL__
+
+/*
+ * early_idt_handler_array is an array of entry points referenced in the
+ * early IDT.  For simplicity, it's a real array with one entry point
+ * every nine bytes.  That leaves room for an optional 'push $0' if the
+ * vector has no error code (two bytes), a 'push $vector_number' (two
+ * bytes), and a jump to the common entry code (up to five bytes).
+ */
+#define EARLY_IDT_HANDLER_SIZE 9
+
 #ifndef __ASSEMBLY__
-extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][2+2+5];
+
+extern const char early_idt_handler_array[NUM_EXCEPTION_VECTORS][EARLY_IDT_HANDLER_SIZE];
 
 /*
  * Load a segment. Fall back on loading the zero

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 1be8e43b669ee4ef405cafe65ececdbb611fce6c..7ad05fd5c51c033a8a07d31a8d9e434e21d617a7 100644 (file)
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -162,7 +162,7 @@ asmlinkage void __init x86_64_start_kernel(char * real_mode_data)
        clear_bss();
 
        for (i = 0; i < NUM_EXCEPTION_VECTORS; i++)
-               set_intr_gate(i, &early_idt_handlers[i]);
+               set_intr_gate(i, &early_idt_handler_array[i]);
        load_idt((const struct desc_ptr *)&idt_descr);
 
        copy_bootdata(__va(real_mode_data));

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index f36bd42d6f0c8b5fc5cd75dcf133b35a1f6bfe3b..30a2aa3782fafd53998d131be23c7564482c5d1c 100644 (file)
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -477,21 +477,22 @@ is486:
 __INIT
 setup_once:
        /*
-        * Set up a idt with 256 entries pointing to ignore_int,
-        * interrupt gates. It doesn't actually load idt - that needs
-        * to be done on each CPU. Interrupts are enabled elsewhere,
-        * when we can be relatively sure everything is ok.
+        * Set up a idt with 256 interrupt gates that push zero if there
+        * is no error code and then jump to early_idt_handler_common.
+        * It doesn't actually load the idt - that needs to be done on
+        * each CPU. Interrupts are enabled elsewhere, when we can be
+        * relatively sure everything is ok.
         */
 
        movl $idt_table,%edi
-       movl $early_idt_handlers,%eax
+       movl $early_idt_handler_array,%eax
        movl $NUM_EXCEPTION_VECTORS,%ecx
 1:
        movl %eax,(%edi)
        movl %eax,4(%edi)
        /* interrupt gate, dpl=0, present */
        movl $(0x8E000000 + __KERNEL_CS),2(%edi)
-       addl $9,%eax
+       addl $EARLY_IDT_HANDLER_SIZE,%eax
        addl $8,%edi
        loop 1b
 
@@ -523,26 +524,28 @@ setup_once:
        andl $0,setup_once_ref  /* Once is enough, thanks */
        ret
 
-ENTRY(early_idt_handlers)
+ENTRY(early_idt_handler_array)
        # 36(%esp) %eflags
        # 32(%esp) %cs
        # 28(%esp) %eip
        # 24(%rsp) error code
        i = 0
        .rept NUM_EXCEPTION_VECTORS
-       .if (EXCEPTION_ERRCODE_MASK >> i) & 1
-       ASM_NOP2
-       .else
+       .ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
        pushl $0                # Dummy error code, to make stack frame uniform
        .endif
        pushl $i                # 20(%esp) Vector number
-       jmp early_idt_handler
+       jmp early_idt_handler_common
        i = i + 1
+       .fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
        .endr
-ENDPROC(early_idt_handlers)
+ENDPROC(early_idt_handler_array)
        
-       /* This is global to keep gas from relaxing the jumps */
-ENTRY(early_idt_handler)
+early_idt_handler_common:
+       /*
+        * The stack is the hardware frame, an error code or zero, and the
+        * vector number.
+        */
        cld
 
        cmpl $2,(%esp)          # X86_TRAP_NMI
@@ -602,7 +605,7 @@ ex_entry:
 is_nmi:
        addl $8,%esp            /* drop vector number and error code */
        iret
-ENDPROC(early_idt_handler)
+ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
        ALIGN

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index a468c0a65c42e00df4e10afd9921d81a53dbba3d..a2dc0add72ed034eed93741811d8481f5866b057 100644 (file)
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -321,26 +321,28 @@ bad_address:
        jmp bad_address
 
        __INIT
-       .globl early_idt_handlers
-early_idt_handlers:
+ENTRY(early_idt_handler_array)
        # 104(%rsp) %rflags
        #  96(%rsp) %cs
        #  88(%rsp) %rip
        #  80(%rsp) error code
        i = 0
        .rept NUM_EXCEPTION_VECTORS
-       .if (EXCEPTION_ERRCODE_MASK >> i) & 1
-       ASM_NOP2
-       .else
+       .ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
        pushq $0                # Dummy error code, to make stack frame uniform
        .endif
        pushq $i                # 72(%rsp) Vector number
-       jmp early_idt_handler
+       jmp early_idt_handler_common
        i = i + 1
+       .fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
        .endr
+ENDPROC(early_idt_handler_array)
 
-/* This is global to keep gas from relaxing the jumps */
-ENTRY(early_idt_handler)
+early_idt_handler_common:
+       /*
+        * The stack is the hardware frame, an error code or zero, and the
+        * vector number.
+        */
        cld
 
        cmpl $2,(%rsp)          # X86_TRAP_NMI
@@ -412,7 +414,7 @@ ENTRY(early_idt_handler)
 is_nmi:
        addq $16,%rsp           # drop vector number and error code
        INTERRUPT_RETURN
-ENDPROC(early_idt_handler)
+ENDPROC(early_idt_handler_common)
 
        __INITDATA