lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 21 May 2015 17:17:32 -0700 From: Andy Lutomirski <luto@...nel.org> To: x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, "H.J. Lu" <hjl.tools@...il.com> Cc: Borislav Petkov <bp@...en8.de>, Jan Beulich <JBeulich@...e.com>, Binutils <binutils@...rceware.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Andy Lutomirski <luto@...nel.org> Subject: [PATCH] x86: Stop relying on magic jmp behavior for early_idt_handlers 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 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. 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_handlers>: 0: 6a 00 pushq $0x0 2: 6a 00 pushq $0x0 4: e9 14 01 00 00 jmpq 11d <early_idt_handler> ... 48: 6a 08 pushq $0x8 4a: e9 d1 00 00 00 jmpq 120 <early_idt_handler> 4f: cc int3 50: cc int3 ... 117: 6a 00 pushq $0x0 119: 6a 1f pushq $0x1f 11b: eb 03 jmp 120 <early_idt_handler> 11d: cc int3 11e: cc int3 11f: cc int3 Signed-off-by: Andy Lutomirski <luto@...nel.org> --- arch/x86/include/asm/segment.h | 13 +++++++++++-- arch/x86/kernel/head_32.S | 12 ++++++------ arch/x86/kernel/head_64.S | 11 ++++++----- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h index 5a9856eb12ba..4bbe0eb24d7e 100644 --- a/arch/x86/include/asm/segment.h +++ b/arch/x86/include/asm/segment.h @@ -231,12 +231,21 @@ #define TLS_SIZE (GDT_ENTRY_TLS_ENTRIES* 8) #ifdef __KERNEL__ -#ifndef __ASSEMBLY__ -extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][2+2+5]; +/* + * early_idt_handlers is an array of entry points. For simplicity, it's + * a real array. We allocate nine bytes for each entry: two one-byte + * push instructions and a five-byte jump in the worst case. + */ +#define EARLY_IDT_HANDLER_STRIDE 9 +#ifndef __ASSEMBLY__ +extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][EARLY_IDT_HANDLER_STRIDE]; #ifdef CONFIG_TRACING # define trace_early_idt_handlers early_idt_handlers #endif +#endif + +#ifndef __ASSEMBLY__ /* * Load a segment. Fall back on loading the zero diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S index d031bad9e07e..43154cd4ad62 100644 --- a/arch/x86/kernel/head_32.S +++ b/arch/x86/kernel/head_32.S @@ -492,7 +492,7 @@ setup_once: movl %eax,4(%edi) /* interrupt gate, dpl=0, present */ movl $(0x8E000000 + __KERNEL_CS),2(%edi) - addl $9,%eax + addl $EARLY_IDT_HANDLER_STRIDE,%eax addl $8,%edi loop 1b @@ -524,6 +524,7 @@ setup_once: andl $0,setup_once_ref /* Once is enough, thanks */ ret +/* Build the early_idt_handlers array */ ENTRY(early_idt_handlers) # 36(%esp) %eflags # 32(%esp) %cs @@ -531,19 +532,18 @@ ENTRY(early_idt_handlers) # 24(%rsp) error code i = 0 .rept NUM_EXCEPTION_VECTORS - .if (EXCEPTION_ERRCODE_MASK >> i) & 1 - ASM_NOP2 - .else + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc + .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 i = i + 1 .endr + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc ENDPROC(early_idt_handlers) - /* This is global to keep gas from relaxing the jumps */ -ENTRY(early_idt_handler) +early_idt_handler: cld cmpl $2,(%esp) # X86_TRAP_NMI diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index ae6588b301c2..c1874059aadf 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -320,6 +320,7 @@ ENDPROC(start_cpu0) bad_address: jmp bad_address +/* Build the early_idt_handlers array */ __INIT .globl early_idt_handlers early_idt_handlers: @@ -329,18 +330,18 @@ early_idt_handlers: # 80(%rsp) error code i = 0 .rept NUM_EXCEPTION_VECTORS - .if (EXCEPTION_ERRCODE_MASK >> i) & 1 - ASM_NOP2 - .else + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc + .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 i = i + 1 .endr + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc +ENDPROC(early_idt_handlers) -/* This is global to keep gas from relaxing the jumps */ -ENTRY(early_idt_handler) +early_idt_handler: cld cmpl $2,(%rsp) # X86_TRAP_NMI -- 2.3.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists