Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758920AbdCVIrX (ORCPT ); Wed, 22 Mar 2017 04:47:23 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:41125 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758778AbdCVIrT (ORCPT ); Wed, 22 Mar 2017 04:47:19 -0400 Date: Wed, 22 Mar 2017 09:47:06 +0100 From: Peter Zijlstra To: Josh Poimboeuf Cc: mingo@kernel.org, tglx@linutronix.de, hpa@zytor.com, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, arjan@linux.intel.com, bp@alien8.de, richard.weinberger@gmail.com Subject: Re: [PATCH 1/5] x86: Implement __WARN using UD0 Message-ID: <20170322084706.GV5680@worktop> References: <20170317211918.393791494@infradead.org> <20170317212350.828368979@infradead.org> <20170321140340.urru2lmjko6txwl5@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170321140340.urru2lmjko6txwl5@treble> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7625 Lines: 291 A little like so then. --- Subject: x86: Implement __WARN using UD0 From: Peter Zijlstra Date: Thu Feb 2 14:43:51 CET 2017 By using "UD0" for WARNs we remove the function call and its possible __FILE__ and __LINE__ immediate arguments from the instruction stream. Total image size will not change much, what we win in the instruction stream we'll loose because of the __bug_table entries. Still, saves on I$ footprint and the total image size does go down a bit. text data bss dec hex filename 10702123 4530992 843776 16076891 f5505b defconfig-build/vmlinux.0 10682460 4530992 843776 16057228 f5038c defconfig-build/vmlinux.1 (um didn't seem to use GENERIC_BUG at all, so remove it) Cc: Linus Torvalds Cc: Richard Weinberger Cc: Arjan van de Ven Cc: Borislav Petkov Cc: Josh Poimboeuf Signed-off-by: Peter Zijlstra (Intel) --- arch/um/Kconfig.common | 5 -- arch/x86/include/asm/bug.h | 73 +++++++++++++++++++++++++++++++---------- arch/x86/kernel/dumpstack.c | 3 - arch/x86/kernel/dumpstack_32.c | 12 ------ arch/x86/kernel/dumpstack_64.c | 10 ----- arch/x86/kernel/traps.c | 46 ++++++++++++++++++++++--- arch/x86/um/Makefile | 2 - arch/x86/um/bug.c | 21 ----------- 8 files changed, 97 insertions(+), 75 deletions(-) --- a/arch/um/Kconfig.common +++ b/arch/um/Kconfig.common @@ -50,11 +50,6 @@ config GENERIC_CALIBRATE_DELAY bool default y -config GENERIC_BUG - bool - default y - depends on BUG - config HZ int default 100 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -1,36 +1,75 @@ #ifndef _ASM_X86_BUG_H #define _ASM_X86_BUG_H +#include + #define HAVE_ARCH_BUG -#ifdef CONFIG_DEBUG_BUGVERBOSE +/* + * Since some emulators terminate on UD2, we cannot use it for WARN. + * Since various instruction decoders disagree on the length of UD1, + * we cannot use it either. So use UD0 for WARN. + * + * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas + * our kernel decoder thinks it takes a ModRM byte, which seems consistent + * with various things like the Intel SDM instruction encoding rules) + */ + +#define ASM_UD0 ".byte 0x0f, 0xff" +#define ASM_UD1 ".byte 0x0f, 0xb9" /* + ModRM */ +#define ASM_UD2 ".byte 0x0f, 0x0b" + +#define INSN_UD0 0xff0f +#define INSN_UD2 0x0b0f + +#define LEN_UD0 2 #ifdef CONFIG_X86_32 -# define __BUG_C0 "2:\t.long 1b, %c0\n" +# define __BUG_REL(val) ".long " __stringify(val) #else -# define __BUG_C0 "2:\t.long 1b - 2b, %c0 - 2b\n" +# define __BUG_REL(val) ".long " __stringify(val) " - 2b" #endif -#define BUG() \ -do { \ - asm volatile("1:\tud2\n" \ - ".pushsection __bug_table,\"a\"\n" \ - __BUG_C0 \ - "\t.word %c1, 0\n" \ - "\t.org 2b+%c2\n" \ - ".popsection" \ - : : "i" (__FILE__), "i" (__LINE__), \ - "i" (sizeof(struct bug_entry))); \ - unreachable(); \ +#ifdef CONFIG_DEBUG_BUGVERBOSE + +#define _BUG_FLAGS(ins, flags) \ +do { \ + asm volatile("1:\t" ins "\n" \ + ".pushsection __bug_table,\"a\"\n" \ + "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \ + "\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \ + "\t.word %c1" "\t# bug_entry::line\n" \ + "\t.word %c2" "\t# bug_entry::flags\n" \ + "\t.org 2b+%c3\n" \ + ".popsection" \ + : : "i" (__FILE__), "i" (__LINE__), \ + "i" (flags), \ + "i" (sizeof(struct bug_entry))); \ } while (0) -#else +#else /* !CONFIG_DEBUG_BUGVERBOSE */ + +#define _BUG_FLAGS(ins, flags) \ +do { \ + asm volatile("1:\t" ins "\n" \ + ".pushsection __bug_table,\"a\"\n" \ + "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \ + "\t.word %c0" "\t# bug_entry::flags\n" \ + "\t.org 2b+%c1\n" \ + ".popsection" \ + : : "i" (flags), \ + "i" (sizeof(struct bug_entry))); \ +} while (0) + +#endif /* CONFIG_DEBUG_BUGVERBOSE */ + #define BUG() \ do { \ - asm volatile("ud2"); \ + _BUG_FLAGS(ASM_UD2, 0); \ unreachable(); \ } while (0) -#endif + +#define __WARN_TAINT(taint) _BUG_FLAGS(ASM_UD0, BUGFLAG_TAINT(taint)) #include --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -289,9 +289,6 @@ void die(const char *str, struct pt_regs unsigned long flags = oops_begin(); int sig = SIGSEGV; - if (!user_mode(regs)) - report_bug(regs->ip, regs); - if (__die(str, regs, err)) sig = 0; oops_end(flags, regs, sig); --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -162,15 +162,3 @@ void show_regs(struct pt_regs *regs) } pr_cont("\n"); } - -int is_valid_bugaddr(unsigned long ip) -{ - unsigned short ud2; - - if (ip < PAGE_OFFSET) - return 0; - if (probe_kernel_address((unsigned short *)ip, ud2)) - return 0; - - return ud2 == 0x0b0f; -} --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -178,13 +178,3 @@ void show_regs(struct pt_regs *regs) } pr_cont("\n"); } - -int is_valid_bugaddr(unsigned long ip) -{ - unsigned short ud2; - - if (__copy_from_user(&ud2, (const void __user *) ip, sizeof(ud2))) - return 0; - - return ud2 == 0x0b0f; -} --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -169,6 +169,37 @@ void ist_end_non_atomic(void) preempt_disable(); } +int is_valid_bugaddr(unsigned long addr) +{ + unsigned short ud; + + if (addr < TASK_SIZE_MAX) + return 0; + + if (probe_kernel_address((unsigned short *)addr, ud)) + return 0; + + return ud == INSN_UD0 || ud == INSN_UD2; +} + +static int fixup_bug(struct pt_regs *regs, int trapnr) +{ + if (trapnr != X86_TRAP_UD) + return 0; + + switch (report_bug(regs->ip, regs)) { + case BUG_TRAP_TYPE_NONE: + case BUG_TRAP_TYPE_BUG: + break; + + case BUG_TRAP_TYPE_WARN: + regs->ip += LEN_UD0; + return 1; + } + + return 0; +} + static nokprobe_inline int do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str, struct pt_regs *regs, long error_code) @@ -187,12 +218,15 @@ do_trap_no_signal(struct task_struct *ts } if (!user_mode(regs)) { - if (!fixup_exception(regs, trapnr)) { - tsk->thread.error_code = error_code; - tsk->thread.trap_nr = trapnr; - die(str, regs, error_code); - } - return 0; + if (fixup_exception(regs, trapnr)) + return 0; + + if (fixup_bug(regs, trapnr)) + return 0; + + tsk->thread.error_code = error_code; + tsk->thread.trap_nr = trapnr; + die(str, regs, error_code); } return -1; --- a/arch/x86/um/Makefile +++ b/arch/x86/um/Makefile @@ -8,7 +8,7 @@ else BITS := 64 endif -obj-y = bug.o bugs_$(BITS).o delay.o fault.o ldt.o \ +obj-y = bugs_$(BITS).o delay.o fault.o ldt.o \ ptrace_$(BITS).o ptrace_user.o setjmp_$(BITS).o signal.o \ stub_$(BITS).o stub_segv.o \ sys_call_table_$(BITS).o sysrq_$(BITS).o tls_$(BITS).o \ --- a/arch/x86/um/bug.c +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright (C) 2006 Jeff Dike (jdike@addtoit.com) - * Licensed under the GPL V2 - */ - -#include - -/* - * Mostly copied from i386/x86_86 - eliminated the eip < PAGE_OFFSET because - * that's not relevant in skas mode. - */ - -int is_valid_bugaddr(unsigned long eip) -{ - unsigned short ud2; - - if (probe_kernel_address((unsigned short __user *)eip, ud2)) - return 0; - - return ud2 == 0x0b0f; -}