[Devel] [PATCH RHEL7 COMMIT] ms/x86/asm/irq: Stop relying on magic JMP behavior for early_idt_handlers

Konstantin Khorenko khorenko at virtuozzo.com
Tue Jun 27 14:56:10 MSK 2017


The commit is pushed to "branch-rh7-3.10.0-514.16.1.vz7.32.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-514.16.1.vz7.32.11
------>
commit 6c8dfa2c1799132947d3c96254a9b0c860cf7d6a
Author: Andy Lutomirski <luto at kernel.org>
Date:   Tue Jun 27 15:56:10 2017 +0400

    ms/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 at kernel.org>
    Acked-by: H. Peter Anvin <hpa at linux.intel.com>
    Cc: Binutils <binutils at sourceware.org>
    Cc: Borislav Petkov <bp at alien8.de>
    Cc: H.J. Lu <hjl.tools at gmail.com>
    Cc: Jan Beulich <JBeulich at suse.com>
    Cc: Linus Torvalds <torvalds at linux-foundation.org>
    Cc: Peter Zijlstra <peterz at infradead.org>
    Cc: Thomas Gleixner <tglx at linutronix.de>
    Link: http://lkml.kernel.org/r/ac027962af343b0c599cbfcf50b945ad2ef3d7a8.1432336324.git.luto@kernel.org
    Signed-off-by: Ingo Molnar <mingo at kernel.org>
    Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
---
 arch/x86/include/asm/segment.h | 14 ++++++++++++--
 arch/x86/kernel/head64.c       |  2 +-
 arch/x86/kernel/head_32.S      | 33 ++++++++++++++++++---------------
 arch/x86/kernel/head_64.S      | 20 +++++++++++---------
 4 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index 6f1c3a8..1919a72 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -212,10 +212,20 @@
 #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];
 #ifdef CONFIG_TRACING
-#define trace_early_idt_handlers early_idt_handlers
+# define trace_early_idt_handler_array early_idt_handler_array
 #endif
 
 /*
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index c2dd757..899ab44 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -166,7 +166,7 @@ void __init x86_64_start_kernel(char * real_mode_data)
 	kasan_early_init();
 
 	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 6ccf14a..7592a9c 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -497,21 +497,22 @@ check_x87:
 __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
 
@@ -543,26 +544,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
@@ -622,7 +625,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 bbc3f59..fb4ca5b 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -329,26 +329,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
@@ -420,7 +422,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
 


More information about the Devel mailing list