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: Richard Weinberger <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/um/Kconfig.common | 5 ---
arch/x86/include/asm/bug.h | 68 ++++++++++++++++++++++++++++++-----------
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 | 50 ++++++++++++++++++++++++++----
arch/x86/um/Makefile | 2 -
arch/x86/um/bug.c | 21 ------------
8 files changed, 96 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,70 @@
#ifndef _ASM_X86_BUG_H
#define _ASM_X86_BUG_H
+#include <linux/stringify.h>
+
#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"
#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 <asm-generic/bug.h>
--- 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,41 @@ void ist_end_non_atomic(void)
preempt_disable();
}
+int is_valid_bugaddr(unsigned long addr)
+{
+ unsigned short ud;
+
+#ifdef CONFIG_X86_32
+ if (addr < PAGE_OFFSET)
+ return 0;
+#else
+ if ((long)addr > 0)
+ return 0;
+#endif
+ if (probe_kernel_address((unsigned short *)addr, ud))
+ return 0;
+
+ return ud == 0x0b0f || ud == 0xff0f;
+}
+
+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 += 2;
+ 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 +222,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 ([email protected])
- * Licensed under the GPL V2
- */
-
-#include <linux/uaccess.h>
-
-/*
- * 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;
-}
On Fri, Mar 17, 2017 at 10:19:19PM +0100, Peter Zijlstra wrote:
> --- a/arch/x86/include/asm/bug.h
> +++ b/arch/x86/include/asm/bug.h
> @@ -1,36 +1,70 @@
> #ifndef _ASM_X86_BUG_H
> #define _ASM_X86_BUG_H
>
> +#include <linux/stringify.h>
> +
> #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"
Thas ASM_UD1 macro isn't used anywhere.
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -169,6 +169,41 @@ void ist_end_non_atomic(void)
> preempt_disable();
> }
>
> +int is_valid_bugaddr(unsigned long addr)
> +{
> + unsigned short ud;
> +
> +#ifdef CONFIG_X86_32
> + if (addr < PAGE_OFFSET)
> + return 0;
> +#else
> + if ((long)addr > 0)
> + return 0;
> +#endif
I think comparing with TASK_SIZE would be more correct and it wouldn't
need an ifdef.
> + if (probe_kernel_address((unsigned short *)addr, ud))
> + return 0;
> +
> + return ud == 0x0b0f || ud == 0xff0f;
> +}
This code would be easier to grok if these were defines IMO.
Also, now that some of the BUG-specific functions are now also related
to WARN, they should probably be renamed to describe their new purpose,
like:
"report_bug" -> "report_bug_or_warning"
"fixup_bug" -> "fixup_bug_or_warning"
On a related note, if warn and bug are going to continue to use two
separate ud instructions for the foreseeable future, report_bug() could
be cleaned up a bit: e.g., for a ud0 instruction, it doesn't make sense
to call find_bug().
> +
> +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 += 2;
> + return 1;
For self-documentation purposes, maybe use a define for the length of
the ud0 instruction?
--
Josh
On 3/21/2017 8:14 AM, Peter Zijlstra wrote:
> For self-documentation purposes, maybe use a define for the length of
> the ud0 instruction?
#define TWO 2
;-)
some things make sense as a define, others don't
(adding a comment, maybe)
On Tue, Mar 21, 2017 at 04:14:46PM +0100, Peter Zijlstra wrote:
> > to WARN, they should probably be renamed to describe their new purpose,
> > like:
> >
> > "report_bug" -> "report_bug_or_warning"
> > "fixup_bug" -> "fixup_bug_or_warning"
> >
> > On a related note, if warn and bug are going to continue to use two
> > separate ud instructions for the foreseeable future, report_bug() could
> > be cleaned up a bit: e.g., for a ud0 instruction, it doesn't make sense
> > to call find_bug().
>
> I'm sure you'll break $random arch if you go futz with that. Also, I
> think you mean UD2, since that's BUG. We actually need the bug_entry for
> WARNs (aka UD0).
>
> Also, you're now optimizing the BUG() code; I don't think anybody cares
> about saving a few cycles there. It shouldn't happen in the first place.
My thinking was to make report_bug() a little less obtuse, but yeah,
that would break other arches, so never mind...
> > > +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 += 2;
> > > + return 1;
> >
> > For self-documentation purposes, maybe use a define for the length of
> > the ud0 instruction?
>
> Well, UD0 and UD2 really. LENGTH_UD0_OR_UD2 is a bit of a fail, name
> wise.
Why UD2? Warnings are UD0-only, no? What about UD0_LEN? Or at least a
comment would be helpful I think.
--
Josh
On Tue, Mar 21, 2017 at 10:32:52AM -0500, Josh Poimboeuf wrote:
> On Tue, Mar 21, 2017 at 04:14:46PM +0100, Peter Zijlstra wrote:
> > > > +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 += 2;
> > > > + return 1;
> > >
> > > For self-documentation purposes, maybe use a define for the length of
> > > the ud0 instruction?
> >
> > Well, UD0 and UD2 really. LENGTH_UD0_OR_UD2 is a bit of a fail, name
> > wise.
>
> Why UD2? Warnings are UD0-only, no? What about UD0_LEN? Or at least a
> comment would be helpful I think.
Oh, right, we only resume on WARN,.. Doh.
On Tue, Mar 21, 2017 at 09:03:40AM -0500, Josh Poimboeuf wrote:
> On Fri, Mar 17, 2017 at 10:19:19PM +0100, Peter Zijlstra wrote:
> > +/*
> > + * 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"
>
> Thas ASM_UD1 macro isn't used anywhere.
I have it there for completeness sake, and documentation purposes.
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -169,6 +169,41 @@ void ist_end_non_atomic(void)
> > preempt_disable();
> > }
> >
> > +int is_valid_bugaddr(unsigned long addr)
> > +{
> > + unsigned short ud;
> > +
> > +#ifdef CONFIG_X86_32
> > + if (addr < PAGE_OFFSET)
> > + return 0;
> > +#else
> > + if ((long)addr > 0)
> > + return 0;
> > +#endif
>
> I think comparing with TASK_SIZE would be more correct and it wouldn't
> need an ifdef.
Dunno about more correct (the TIF_ADDR32 case is irrelevant here), but
yes, that would do away with the #ifdef.
> > + if (probe_kernel_address((unsigned short *)addr, ud))
> > + return 0;
> > +
> > + return ud == 0x0b0f || ud == 0xff0f;
> > +}
>
> This code would be easier to grok if these were defines IMO.
Would be nice if I could use the same defines; but I can't see how I can
make that happen :/ But sure; I suppose I can add more defines.
> Also, now that some of the BUG-specific functions are now also related
They already were, its just that x86 only now will use the WARN part of
it. I don't feel that should be part of this patch.
> to WARN, they should probably be renamed to describe their new purpose,
> like:
>
> "report_bug" -> "report_bug_or_warning"
> "fixup_bug" -> "fixup_bug_or_warning"
>
> On a related note, if warn and bug are going to continue to use two
> separate ud instructions for the foreseeable future, report_bug() could
> be cleaned up a bit: e.g., for a ud0 instruction, it doesn't make sense
> to call find_bug().
I'm sure you'll break $random arch if you go futz with that. Also, I
think you mean UD2, since that's BUG. We actually need the bug_entry for
WARNs (aka UD0).
Also, you're now optimizing the BUG() code; I don't think anybody cares
about saving a few cycles there. It shouldn't happen in the first place.
> > +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 += 2;
> > + return 1;
>
> For self-documentation purposes, maybe use a define for the length of
> the ud0 instruction?
Well, UD0 and UD2 really. LENGTH_UD0_OR_UD2 is a bit of a fail, name
wise.
A little like so then.
---
Subject: x86: Implement __WARN using UD0
From: Peter Zijlstra <[email protected]>
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 <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
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 <linux/stringify.h>
+
#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 <asm-generic/bug.h>
--- 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 ([email protected])
- * Licensed under the GPL V2
- */
-
-#include <linux/uaccess.h>
-
-/*
- * 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;
-}
On Wed, Mar 22, 2017 at 09:47:06AM +0100, Peter Zijlstra wrote:
>
> A little like so then.
>
> ---
> Subject: x86: Implement __WARN using UD0
> From: Peter Zijlstra <[email protected]>
> 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 <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: Arjan van de Ven <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Josh Poimboeuf <[email protected]>
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Looks good, thanks!
Reviewed-by: Josh Poimboeuf <[email protected]>
--
Josh