Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1301205 > unrolled thread

[PATCH v7 1/3] x86: Add classes to exception tables

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v7 1/3] x86: Add classes to exception tables Tony Luck <tony.luck@intel.com> - 2016-01-05 01:20 +0100
    Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-06 13:40 +0100
      Re: [PATCH v7 1/3] x86: Add classes to exception tables "Luck, Tony" <tony.luck@intel.com> - 2016-01-06 18:40 +0100
        Re: [PATCH v7 1/3] x86: Add classes to exception tables Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-06 18:50 +0100
      Re: [PATCH v7 1/3] x86: Add classes to exception tables Andy Lutomirski <luto@amacapital.net> - 2016-01-06 19:00 +0100
        Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-06 19:10 +0100
          Re: [PATCH v7 1/3] x86: Add classes to exception tables Andy Lutomirski <luto@amacapital.net> - 2016-01-06 19:10 +0100
            Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-06 20:50 +0100
              Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-07 13:20 +0100
                Re: [PATCH v7 1/3] x86: Add classes to exception tables "Luck, Tony" <tony.luck@intel.com> - 2016-01-07 19:30 +0100
                Re: [PATCH v7 1/3] x86: Add classes to exception tables "Luck, Tony" <tony.luck@intel.com> - 2016-01-08 02:50 +0100
                  Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-08 11:40 +0100
                    RE: [PATCH v7 1/3] x86: Add classes to exception tables "Luck, Tony" <tony.luck@intel.com> - 2016-01-08 17:30 +0100
                      Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-08 18:30 +0100
                      Re: [PATCH v7 1/3] x86: Add classes to exception tables Brian Gerst <brgerst@gmail.com> - 2016-01-08 23:30 +0100
                Re: [PATCH v7 1/3] x86: Add classes to exception tables "Luck, Tony" <tony.luck@intel.com> - 2016-01-08 06:40 +0100
                  Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-08 11:50 +0100
    Re: [PATCH v7 1/3] x86: Add classes to exception tables Borislav Petkov <bp@alien8.de> - 2016-01-06 13:40 +0100

#1301205 — [PATCH v7 1/3] x86: Add classes to exception tables

Starting with a patch from Andy Lutomirski <luto@amacapital.net>
that used linker relocation trickery to free up a couple of bits
in the "fixup" field of the exception table (and generalized the
uaccess_err hack to use one of the classes).

This patch allocates another one of the classes to provide
a mechanism to provide the fault number to the fixup code
in %rax.

Still one free class for the next brilliant idea. If more are
needed it should be possible to squeeze another bit or three
extending the same technique.

Originally-from: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/asm.h     | 102 +++++++++++++++++++++++++++++++----------
 arch/x86/include/asm/uaccess.h |  17 +++++--
 arch/x86/kernel/kprobes/core.c |   2 +-
 arch/x86/kernel/traps.c        |   6 +--
 arch/x86/mm/extable.c          |  66 ++++++++++++++++++--------
 arch/x86/mm/fault.c            |   2 +-
 6 files changed, 142 insertions(+), 53 deletions(-)

diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 189679aba703..977273e36f87 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -43,19 +43,79 @@
 #define _ASM_DI		__ASM_REG(di)
 
 /* Exception table entry */
-#ifdef __ASSEMBLY__
-# define _ASM_EXTABLE(from,to)					\
-	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
-	.long (from) - . ;					\
-	.long (to) - . ;					\
-	.popsection
 
-# define _ASM_EXTABLE_EX(from,to)				\
-	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
-	.long (from) - . ;					\
-	.long (to) - . + 0x7ffffff0 ;				\
+#define	_EXTABLE_BIAS	0x20000000
+/*
+ * An exception table entry is 64 bits.  The first 32 bits are the offset
+ * from that entry to the potentially faulting instruction.  sortextable
+ * relies on that exact encoding.  The second 32 bits encode the fault
+ * handler address.
+ *
+ * We want to stick two extra bits of handler class into the fault handler
+ * address.  All of these are generated by relocations, so we can only
+ * rely on addition.
+ *
+ * The offset to the fixup is signed, and we're trying to use the high
+ * bits for a different purpose.  In C, we could just do:
+ *
+ * u32 class_and_offset = ((target - here) & 0x3fffffff) | class;
+ *
+ * Then, to decode it, we'd mask off the class and sign-extend to recover
+ * the offset.
+ *
+ * In asm, we can't do that, because this all gets laundered through the
+ * linker, and there's no relocation type that supports this chicanery.
+ * Instead we cheat a bit.  We first add a large number to the offset
+ * (0x20000000).  The result is still nominally signed, but now it's
+ * always positive, and the two high bits are always clear.  We can then
+ * set high bits by ordinary addition or subtraction instead of using
+ * bitwise operations.  As far as the linker is concerned, all we're
+ * doing is adding a large constant to the difference between here (".")
+ * and the target, and that's a valid relocation type.
+ *
+ * We therefore emit:
+ * (target - here) + (class) + 0x20000000
+ *
+ * This has the property that the two high bits are the class (see
+ * ex_class().
+ *
+ * To get the offset back we just mask off the class bits and subtract
+ * 0x20000000. See ex_fixup_addr().
+ */
+
+/*
+ * There are two bits of extable entry class giving four classes
+ */
+#define EXTABLE_CLASS_DEFAULT	0	/* standard uaccess fixup */
+#define EXTABLE_CLASS_FAULT	1	/* provide fault number as well as fixup */
+#define EXTABLE_CLASS_EX	2	/* uaccess + set uaccess_err */
+#define EXTABLE_CLASS_UNUSED	3	/* available for something else */
+
+/*
+ * The biases are the class constants + _EXTABLE_BIAS, as signed integers.
+ * This can't use ordinary arithmetic -- the assembler isn't that smart.
+ */
+#define _EXTABLE_BIAS_DEFAULT	_EXTABLE_BIAS
+#define _EXTABLE_BIAS_FAULT	_EXTABLE_BIAS + 0x40000000
+#define _EXTABLE_BIAS_EX	_EXTABLE_BIAS - 0x80000000
+#define _EXTABLE_BIAS_UNUSED	_EXTABLE_BIAS - 0x40000000
+
+#define _ASM_EXTABLE(from,to)						\
+	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_DEFAULT)
+
+#define _ASM_EXTABLE_FAULT(from,to)					\
+	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_FAULT)
+
+#define _ASM_EXTABLE_EX(from,to)					\
+	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_EX)
+
+#ifdef __ASSEMBLY__
+# define _EXPAND_EXTABLE_BIAS(x) x
+# define _ASM_EXTABLE_CLASS(from,to,bias)				\
+	.pushsection "__ex_table","a" ;					\
+	.balign 8 ;							\
+	.long (from) - . ;						\
+	.long (to) - . + _EXPAND_EXTABLE_BIAS(bias) ;			\
 	.popsection
 
 # define _ASM_NOKPROBE(entry)					\
@@ -89,18 +149,12 @@
 	.endm
 
 #else
-# define _ASM_EXTABLE(from,to)					\
-	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
-	" .long (" #from ") - .\n"				\
-	" .long (" #to ") - .\n"				\
-	" .popsection\n"
-
-# define _ASM_EXTABLE_EX(from,to)				\
-	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
-	" .long (" #from ") - .\n"				\
-	" .long (" #to ") - . + 0x7ffffff0\n"			\
+# define _EXPAND_EXTABLE_BIAS(x) #x
+# define _ASM_EXTABLE_CLASS(from,to,bias)				\
+	" .pushsection \"__ex_table\",\"a\"\n"				\
+	" .balign 8\n"							\
+	" .long (" #from ") - .\n"					\
+	" .long (" #to ") - . + " _EXPAND_EXTABLE_BIAS(bias) "\n"	\
 	" .popsection\n"
 /* For C file, we already have NOKPROBE_SYMBOL macro */
 #endif
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a8df874f3e88..b023300cd6f0 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -93,9 +93,12 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
  * The exception table consists of pairs of addresses relative to the
  * exception table enty itself: the first is the address of an
  * instruction that is allowed to fault, and the second is the address
- * at which the program should continue.  No registers are modified,
- * so it is entirely up to the continuation code to figure out what to
- * do.
+ * at which the program should continue.  The exception table is linked
+ * soon after the fixup section, so we don't need a full 32-bit offset
+ * for the fixup. We steal the two upper bits so we can tag exception
+ * table entries with different classes. In the default class no registers
+ * are modified, so it is entirely up to the continuation code to figure
+ * out what to do.
  *
  * All the routines below use bits of fixup code that are out of line
  * with the main instruction path.  This means when everything is well,
@@ -110,7 +113,13 @@ struct exception_table_entry {
 #define ARCH_HAS_SORT_EXTABLE
 #define ARCH_HAS_SEARCH_EXTABLE
 
-extern int fixup_exception(struct pt_regs *regs);
+static inline unsigned int
+ex_class(const struct exception_table_entry *x)
+{
+	return (u32)x->fixup >> 30;
+}
+
+extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern int early_fixup_exception(unsigned long *ip);
 
 /*
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 1deffe6cc873..0f05deeff5ce 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -988,7 +988,7 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 		 * In case the user-specified fault handler returned
 		 * zero, try to fix up.
 		 */
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, trapnr))
 			return 1;
 
 		/*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 346eec73f7db..df25081e5970 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -199,7 +199,7 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str,
 	}
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			tsk->thread.error_code = error_code;
 			tsk->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
@@ -453,7 +453,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
 
 	tsk = current;
 	if (!user_mode(regs)) {
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, X86_TRAP_GP))
 			return;
 
 		tsk->thread.error_code = error_code;
@@ -699,7 +699,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	conditional_sti(regs);
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, X86_TRAP_DE)) {
 			task->thread.error_code = error_code;
 			task->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 903ec1e9c326..7a592ec193d5 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -11,13 +11,51 @@ ex_insn_addr(const struct exception_table_entry *x)
 static inline unsigned long
 ex_fixup_addr(const struct exception_table_entry *x)
 {
-	return (unsigned long)&x->fixup + x->fixup;
+	long offset = (long)((u32)x->fixup & 0x3fffffff) - (long)_EXTABLE_BIAS;
+	return (unsigned long)&x->fixup + offset;
 }
 
-int fixup_exception(struct pt_regs *regs)
+/* Fixup functions for each exception class */
+static int fix_class_default(const struct exception_table_entry *fixup,
+		      struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+static int fix_class_fault(const struct exception_table_entry *fixup,
+		      struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+	regs->ax = trapnr;
+	return 1;
+}
+static int fix_class_ex(const struct exception_table_entry *fixup,
+		      struct pt_regs *regs, int trapnr)
+{
+	/* Special hack for uaccess_err */
+	current_thread_info()->uaccess_err = 1;
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+static int fix_class_unused(const struct exception_table_entry *fixup,
+		      struct pt_regs *regs, int trapnr)
+{
+	/* can't happen unless exception table was corrupted */
+	BUG_ON(1);
+	return 0;
+}
+
+static int (*allclasses[])(const struct exception_table_entry *,
+			   struct pt_regs *, int) = {
+	[EXTABLE_CLASS_DEFAULT] = fix_class_default,
+	[EXTABLE_CLASS_FAULT] = fix_class_fault,
+	[EXTABLE_CLASS_EX] = fix_class_ex,
+	[EXTABLE_CLASS_UNUSED] = fix_class_unused
+};
+
+int fixup_exception(struct pt_regs *regs, int trapnr)
 {
 	const struct exception_table_entry *fixup;
-	unsigned long new_ip;
 
 #ifdef CONFIG_PNPBIOS
 	if (unlikely(SEGMENT_IS_PNP_CODE(regs->cs))) {
@@ -34,17 +72,8 @@ int fixup_exception(struct pt_regs *regs)
 #endif
 
 	fixup = search_exception_tables(regs->ip);
-	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
-
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* Special hack for uaccess_err */
-			current_thread_info()->uaccess_err = 1;
-			new_ip -= 0x7ffffff0;
-		}
-		regs->ip = new_ip;
-		return 1;
-	}
+	if (fixup)
+		return allclasses[ex_class(fixup)](fixup, regs, trapnr);
 
 	return 0;
 }
@@ -53,18 +82,15 @@ int fixup_exception(struct pt_regs *regs)
 int __init early_fixup_exception(unsigned long *ip)
 {
 	const struct exception_table_entry *fixup;
-	unsigned long new_ip;
 
 	fixup = search_exception_tables(*ip);
 	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
-
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* uaccess handling not supported during early boot */
+		if (ex_class(fixup)) {
+			/* special handling not supported during early boot */
 			return 0;
 		}
 
-		*ip = new_ip;
+		*ip = ex_fixup_addr(fixup);
 		return 1;
 	}
 
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index eef44d9a3f77..495946c3f9dd 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -656,7 +656,7 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 	int sig;
 
 	/* Are we prepared to handle this kernel fault? */
-	if (fixup_exception(regs)) {
+	if (fixup_exception(regs, X86_TRAP_PF)) {
 		/*
 		 * Any interrupt that takes a fault gets the fixup. This makes
 		 * the below recursive fault logic only apply to a faults from
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1302722

On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
> Starting with a patch from Andy Lutomirski <luto@amacapital.net>
> that used linker relocation trickery to free up a couple of bits
> in the "fixup" field of the exception table (and generalized the
> uaccess_err hack to use one of the classes).

So I still think that the other idea Andy gave with putting the handler
in the exception table is much cleaner and straightforward.

Here's a totally untested patch which at least builds here. I think this
approach is much more extensible and simpler for the price of a couple
of KBs of __ex_table size.

---
diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 189679aba703..43b509c88b13 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -44,18 +44,20 @@
 
 /* Exception table entry */
 #ifdef __ASSEMBLY__
-# define _ASM_EXTABLE(from,to)					\
+# define _ASM_EXTABLE(from,to)				\
 	.pushsection "__ex_table","a" ;				\
 	.balign 8 ;						\
 	.long (from) - . ;					\
 	.long (to) - . ;					\
+	.long 0 - .;						\
 	.popsection
 
 # define _ASM_EXTABLE_EX(from,to)				\
 	.pushsection "__ex_table","a" ;				\
 	.balign 8 ;						\
 	.long (from) - . ;					\
-	.long (to) - . + 0x7ffffff0 ;				\
+	.long (to) - . ;					\
+	.long ex_handler_ext - . ;				\
 	.popsection
 
 # define _ASM_NOKPROBE(entry)					\
@@ -94,13 +96,14 @@
 	" .balign 8\n"						\
 	" .long (" #from ") - .\n"				\
 	" .long (" #to ") - .\n"				\
+	" .long 0 - .\n"					\
 	" .popsection\n"
 
 # define _ASM_EXTABLE_EX(from,to)				\
 	" .pushsection \"__ex_table\",\"a\"\n"			\
 	" .balign 8\n"						\
 	" .long (" #from ") - .\n"				\
-	" .long (" #to ") - . + 0x7ffffff0\n"			\
+	" .long ex_handler_ext - .\n"				\
 	" .popsection\n"
 /* For C file, we already have NOKPROBE_SYMBOL macro */
 #endif
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 09b1b0ab94b7..22b49c3b311a 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -104,13 +104,13 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
  */
 
 struct exception_table_entry {
-	int insn, fixup;
+	int insn, fixup, handler;
 };
 /* This is not the generic standard exception_table_entry format */
 #define ARCH_HAS_SORT_EXTABLE
 #define ARCH_HAS_SEARCH_EXTABLE
 
-extern int fixup_exception(struct pt_regs *regs);
+extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern int early_fixup_exception(unsigned long *ip);
 
 /*
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 1deffe6cc873..0f05deeff5ce 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -988,7 +988,7 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 		 * In case the user-specified fault handler returned
 		 * zero, try to fix up.
 		 */
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, trapnr))
 			return 1;
 
 		/*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index ade185a46b1d..211c11c7bba4 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -199,7 +199,7 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str,
 	}
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			tsk->thread.error_code = error_code;
 			tsk->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
@@ -453,7 +453,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
 
 	tsk = current;
 	if (!user_mode(regs)) {
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, X86_TRAP_GP))
 			return;
 
 		tsk->thread.error_code = error_code;
@@ -699,7 +699,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	conditional_sti(regs);
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			task->thread.error_code = error_code;
 			task->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 903ec1e9c326..191f4b7d1d2d 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -3,6 +3,8 @@
 #include <linux/sort.h>
 #include <asm/uaccess.h>
 
+typedef int (*ex_handler_t)(const struct exception_table_entry *, struct pt_regs *, int);
+
 static inline unsigned long
 ex_insn_addr(const struct exception_table_entry *x)
 {
@@ -14,10 +16,39 @@ ex_fixup_addr(const struct exception_table_entry *x)
 	return (unsigned long)&x->fixup + x->fixup;
 }
 
-int fixup_exception(struct pt_regs *regs)
+inline ex_handler_t ex_fixup_handler(const struct exception_table_entry *x)
+{
+	return (ex_handler_t)&x->handler + x->handler;
+}
+
+int ex_handler_default(const struct exception_table_entry *fixup,
+		       struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+
+int ex_handler_fault(const struct exception_table_entry *fixup,
+		     struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+	regs->ax = trapnr;
+	return 1;
+}
+int ex_handler_ext(const struct exception_table_entry *fixup,
+		   struct pt_regs *regs, int trapnr)
 {
-	const struct exception_table_entry *fixup;
+	/* Special hack for uaccess_err */
+	current_thread_info()->uaccess_err = 1;
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+
+int fixup_exception(struct pt_regs *regs, int trapnr)
+{
+	const struct exception_table_entry *e;
 	unsigned long new_ip;
+	ex_handler_t handler;
 
 #ifdef CONFIG_PNPBIOS
 	if (unlikely(SEGMENT_IS_PNP_CODE(regs->cs))) {
@@ -33,42 +64,40 @@ int fixup_exception(struct pt_regs *regs)
 	}
 #endif
 
-	fixup = search_exception_tables(regs->ip);
-	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
-
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* Special hack for uaccess_err */
-			current_thread_info()->uaccess_err = 1;
-			new_ip -= 0x7ffffff0;
-		}
-		regs->ip = new_ip;
-		return 1;
-	}
+	e = search_exception_tables(regs->ip);
+	if (!e)
+		return 0;
+
+	new_ip  = ex_fixup_addr(e);
+	handler = ex_fixup_handler(e);
+
+	if (!handler)
+		handler = ex_handler_default;
+
+	return handler(e, regs, trapnr);
 
-	return 0;
 }
 
 /* Restricted version used during very early boot */
 int __init early_fixup_exception(unsigned long *ip)
 {
-	const struct exception_table_entry *fixup;
+	const struct exception_table_entry *e;
 	unsigned long new_ip;
+	ex_handler_t handler;
 
-	fixup = search_exception_tables(*ip);
-	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
+	e = search_exception_tables(*ip);
+	if (!e)
+		return 0;
 
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* uaccess handling not supported during early boot */
-			return 0;
-		}
+	new_ip  = ex_fixup_addr(e);
+	handler = ex_fixup_handler(e);
 
-		*ip = new_ip;
-		return 1;
-	}
+	/* uaccess handling not supported during early boot */
+	if (handler && handler == ex_handler_ext)
+		return 0;
 
-	return 0;
+	*ip = new_ip;
+	return 1;
 }
 
 /*
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index eef44d9a3f77..495946c3f9dd 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -656,7 +656,7 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 	int sig;
 
 	/* Are we prepared to handle this kernel fault? */
-	if (fixup_exception(regs)) {
+	if (fixup_exception(regs, X86_TRAP_PF)) {
 		/*
 		 * Any interrupt that takes a fault gets the fixup. This makes
 		 * the below recursive fault logic only apply to a faults from

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302920

On Wed, Jan 06, 2016 at 01:33:46PM +0100, Borislav Petkov wrote:
> On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
> > Starting with a patch from Andy Lutomirski <luto@amacapital.net>
> > that used linker relocation trickery to free up a couple of bits
> > in the "fixup" field of the exception table (and generalized the
> > uaccess_err hack to use one of the classes).
> 
> So I still think that the other idea Andy gave with putting the handler
> in the exception table is much cleaner and straightforward.
> 
> Here's a totally untested patch which at least builds here. I think this
> approach is much more extensible and simpler for the price of a couple
> of KBs of __ex_table size.

On my config (based on an enterprise distro config) the vmlinux
ex_table would grow from 8k to 12k.  There are a handful of modules
that would also see 50% expansion (the size is the first hex field after
the __ex_table):

arch/x86/kernel/test_nx.ko 4 __ex_table 00000008 0000000000000000 0000000000000000 000002a8 2**3
arch/x86/kvm/kvm-intel.ko 7 __ex_table 000009b8 0000000000000000 0000000000000000 0001ff60 2**3
arch/x86/kvm/kvm-amd.ko 6 __ex_table 00000070 0000000000000000 0000000000000000 0000a1e0 2**3
arch/x86/kvm/kvm.ko 23 __ex_table 00000080 0000000000000000 0000000000000000 00059940 2**3
drivers/char/ipmi/ipmi_devintf.ko 9 __ex_table 00000098 0000000000000000 0000000000000000 000017f0 2**3
drivers/gpu/drm/i915/i915.ko 20 __ex_table 00000060 0000000000000000 0000000000000000 000f8f10 2**3
drivers/gpu/drm/radeon/radeon.ko 19 __ex_table 000001d0 0000000000000000 0000000000000000 00145c88 2**3
drivers/gpu/drm/drm.ko 24 __ex_table 00000400 0000000000000000 0000000000000000 0003b688 2**3
drivers/media/usb/uvc/uvcvideo.ko 15 __ex_table 00000030 0000000000000000 0000000000000000 0000f100 2**3
drivers/scsi/sg.ko 12 __ex_table 00000060 0000000000000000 0000000000000000 00006548 2**3
drivers/vhost/vhost.ko 12 __ex_table 00000068 0000000000000000 0000000000000000 00003360 2**3
drivers/xen/xen-privcmd.ko 11 __ex_table 00000010 0000000000000000 0000000000000000 00000f70 2**3
net/sunrpc/sunrpc.ko 24 __ex_table 00000008 0000000000000000 0000000000000000 000321b0 2**3
sound/core/snd-pcm.ko 19 __ex_table 00000040 0000000000000000 0000000000000000 000108f0 2**3

Linus, Peter, Ingo, Thomas: Can we head this direction? The code is cleaner
and more flexible. Or should we stick with Andy's clever way to squeeze a
couple of "class" bits into the fixup field of the exception table?

-Tony

> 
> ---
> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> index 189679aba703..43b509c88b13 100644
> --- a/arch/x86/include/asm/asm.h
> +++ b/arch/x86/include/asm/asm.h
> @@ -44,18 +44,20 @@
>  
>  /* Exception table entry */
>  #ifdef __ASSEMBLY__
> -# define _ASM_EXTABLE(from,to)					\
> +# define _ASM_EXTABLE(from,to)				\
>  	.pushsection "__ex_table","a" ;				\
>  	.balign 8 ;						\
>  	.long (from) - . ;					\
>  	.long (to) - . ;					\
> +	.long 0 - .;						\
>  	.popsection
>  
>  # define _ASM_EXTABLE_EX(from,to)				\
>  	.pushsection "__ex_table","a" ;				\
>  	.balign 8 ;						\
>  	.long (from) - . ;					\
> -	.long (to) - . + 0x7ffffff0 ;				\
> +	.long (to) - . ;					\
> +	.long ex_handler_ext - . ;				\
>  	.popsection
>  
>  # define _ASM_NOKPROBE(entry)					\
> @@ -94,13 +96,14 @@
>  	" .balign 8\n"						\
>  	" .long (" #from ") - .\n"				\
>  	" .long (" #to ") - .\n"				\
> +	" .long 0 - .\n"					\
>  	" .popsection\n"
>  
>  # define _ASM_EXTABLE_EX(from,to)				\
>  	" .pushsection \"__ex_table\",\"a\"\n"			\
>  	" .balign 8\n"						\
>  	" .long (" #from ") - .\n"				\
> -	" .long (" #to ") - . + 0x7ffffff0\n"			\
> +	" .long ex_handler_ext - .\n"				\
>  	" .popsection\n"
>  /* For C file, we already have NOKPROBE_SYMBOL macro */
>  #endif
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index 09b1b0ab94b7..22b49c3b311a 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -104,13 +104,13 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
>   */
>  
>  struct exception_table_entry {
> -	int insn, fixup;
> +	int insn, fixup, handler;
>  };
>  /* This is not the generic standard exception_table_entry format */
>  #define ARCH_HAS_SORT_EXTABLE
>  #define ARCH_HAS_SEARCH_EXTABLE
>  
> -extern int fixup_exception(struct pt_regs *regs);
> +extern int fixup_exception(struct pt_regs *regs, int trapnr);
>  extern int early_fixup_exception(unsigned long *ip);
>  
>  /*
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 1deffe6cc873..0f05deeff5ce 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -988,7 +988,7 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  		 * In case the user-specified fault handler returned
>  		 * zero, try to fix up.
>  		 */
> -		if (fixup_exception(regs))
> +		if (fixup_exception(regs, trapnr))
>  			return 1;
>  
>  		/*
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index ade185a46b1d..211c11c7bba4 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -199,7 +199,7 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str,
>  	}
>  
>  	if (!user_mode(regs)) {
> -		if (!fixup_exception(regs)) {
> +		if (!fixup_exception(regs, trapnr)) {
>  			tsk->thread.error_code = error_code;
>  			tsk->thread.trap_nr = trapnr;
>  			die(str, regs, error_code);
> @@ -453,7 +453,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
>  
>  	tsk = current;
>  	if (!user_mode(regs)) {
> -		if (fixup_exception(regs))
> +		if (fixup_exception(regs, X86_TRAP_GP))
>  			return;
>  
>  		tsk->thread.error_code = error_code;
> @@ -699,7 +699,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
>  	conditional_sti(regs);
>  
>  	if (!user_mode(regs)) {
> -		if (!fixup_exception(regs)) {
> +		if (!fixup_exception(regs, trapnr)) {
>  			task->thread.error_code = error_code;
>  			task->thread.trap_nr = trapnr;
>  			die(str, regs, error_code);
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 903ec1e9c326..191f4b7d1d2d 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -3,6 +3,8 @@
>  #include <linux/sort.h>
>  #include <asm/uaccess.h>
>  
> +typedef int (*ex_handler_t)(const struct exception_table_entry *, struct pt_regs *, int);
> +
>  static inline unsigned long
>  ex_insn_addr(const struct exception_table_entry *x)
>  {
> @@ -14,10 +16,39 @@ ex_fixup_addr(const struct exception_table_entry *x)
>  	return (unsigned long)&x->fixup + x->fixup;
>  }
>  
> -int fixup_exception(struct pt_regs *regs)
> +inline ex_handler_t ex_fixup_handler(const struct exception_table_entry *x)
> +{
> +	return (ex_handler_t)&x->handler + x->handler;
> +}
> +
> +int ex_handler_default(const struct exception_table_entry *fixup,
> +		       struct pt_regs *regs, int trapnr)
> +{
> +	regs->ip = ex_fixup_addr(fixup);
> +	return 1;
> +}
> +
> +int ex_handler_fault(const struct exception_table_entry *fixup,
> +		     struct pt_regs *regs, int trapnr)
> +{
> +	regs->ip = ex_fixup_addr(fixup);
> +	regs->ax = trapnr;
> +	return 1;
> +}
> +int ex_handler_ext(const struct exception_table_entry *fixup,
> +		   struct pt_regs *regs, int trapnr)
>  {
> -	const struct exception_table_entry *fixup;
> +	/* Special hack for uaccess_err */
> +	current_thread_info()->uaccess_err = 1;
> +	regs->ip = ex_fixup_addr(fixup);
> +	return 1;
> +}
> +
> +int fixup_exception(struct pt_regs *regs, int trapnr)
> +{
> +	const struct exception_table_entry *e;
>  	unsigned long new_ip;
> +	ex_handler_t handler;
>  
>  #ifdef CONFIG_PNPBIOS
>  	if (unlikely(SEGMENT_IS_PNP_CODE(regs->cs))) {
> @@ -33,42 +64,40 @@ int fixup_exception(struct pt_regs *regs)
>  	}
>  #endif
>  
> -	fixup = search_exception_tables(regs->ip);
> -	if (fixup) {
> -		new_ip = ex_fixup_addr(fixup);
> -
> -		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
> -			/* Special hack for uaccess_err */
> -			current_thread_info()->uaccess_err = 1;
> -			new_ip -= 0x7ffffff0;
> -		}
> -		regs->ip = new_ip;
> -		return 1;
> -	}
> +	e = search_exception_tables(regs->ip);
> +	if (!e)
> +		return 0;
> +
> +	new_ip  = ex_fixup_addr(e);
> +	handler = ex_fixup_handler(e);
> +
> +	if (!handler)
> +		handler = ex_handler_default;
> +
> +	return handler(e, regs, trapnr);
>  
> -	return 0;
>  }
>  
>  /* Restricted version used during very early boot */
>  int __init early_fixup_exception(unsigned long *ip)
>  {
> -	const struct exception_table_entry *fixup;
> +	const struct exception_table_entry *e;
>  	unsigned long new_ip;
> +	ex_handler_t handler;
>  
> -	fixup = search_exception_tables(*ip);
> -	if (fixup) {
> -		new_ip = ex_fixup_addr(fixup);
> +	e = search_exception_tables(*ip);
> +	if (!e)
> +		return 0;
>  
> -		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
> -			/* uaccess handling not supported during early boot */
> -			return 0;
> -		}
> +	new_ip  = ex_fixup_addr(e);
> +	handler = ex_fixup_handler(e);
>  
> -		*ip = new_ip;
> -		return 1;
> -	}
> +	/* uaccess handling not supported during early boot */
> +	if (handler && handler == ex_handler_ext)
> +		return 0;
>  
> -	return 0;
> +	*ip = new_ip;
> +	return 1;
>  }
>  
>  /*
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index eef44d9a3f77..495946c3f9dd 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -656,7 +656,7 @@ no_context(struct pt_regs *regs, unsigned long error_code,
>  	int sig;
>  
>  	/* Are we prepared to handle this kernel fault? */
> -	if (fixup_exception(regs)) {
> +	if (fixup_exception(regs, X86_TRAP_PF)) {
>  		/*
>  		 * Any interrupt that takes a fault gets the fixup. This makes
>  		 * the below recursive fault logic only apply to a faults from
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302926

On Wed, Jan 6, 2016 at 9:35 AM, Luck, Tony <tony.luck@intel.com> wrote:
>
> Linus, Peter, Ingo, Thomas: Can we head this direction? The code is cleaner
> and more flexible. Or should we stick with Andy's clever way to squeeze a
> couple of "class" bits into the fixup field of the exception table?

I'd rather not be clever in order to save just a tiny amount of space
in the exception table, which isn't really criticial for anybody.

So I think Borislav's patch has the advantage of being pretty
straightforward and allowing arbitrary fixups, in case we end up
having localized special cases..

               Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302933

On Wed, Jan 6, 2016 at 4:33 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
>> Starting with a patch from Andy Lutomirski <luto@amacapital.net>
>> that used linker relocation trickery to free up a couple of bits
>> in the "fixup" field of the exception table (and generalized the
>> uaccess_err hack to use one of the classes).
>
> So I still think that the other idea Andy gave with putting the handler
> in the exception table is much cleaner and straightforward.
>
> Here's a totally untested patch which at least builds here. I think this
> approach is much more extensible and simpler for the price of a couple
> of KBs of __ex_table size.
>
> ---
> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> index 189679aba703..43b509c88b13 100644
> --- a/arch/x86/include/asm/asm.h
> +++ b/arch/x86/include/asm/asm.h
> @@ -44,18 +44,20 @@
>
>  /* Exception table entry */
>  #ifdef __ASSEMBLY__
> -# define _ASM_EXTABLE(from,to)                                 \
> +# define _ASM_EXTABLE(from,to)                         \
>         .pushsection "__ex_table","a" ;                         \
>         .balign 8 ;                                             \
>         .long (from) - . ;                                      \
>         .long (to) - . ;                                        \
> +       .long 0 - .;                                            \

I assume that this zero is to save the couple of bytes for the
relocation entry on relocatable kernels?

If so, ...

> +inline ex_handler_t ex_fixup_handler(const struct exception_table_entry *x)
> +{
> +       return (ex_handler_t)&x->handler + x->handler;

I would check for zero here, because...

> +       new_ip  = ex_fixup_addr(e);
> +       handler = ex_fixup_handler(e);
> +
> +       if (!handler)
> +               handler = ex_handler_default;

the !handler condition here will never trigger because the offset was
already applied.

Otherwise this looks generally sane.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302936

On Wed, Jan 06, 2016 at 09:54:19AM -0800, Andy Lutomirski wrote:
> I assume that this zero is to save the couple of bytes for the
> relocation entry on relocatable kernels?

I didn't want to touch all _ASM_EXTABLE() macro invocations by adding a
third param @handler which is redundant as we know which it is.

> > +       new_ip  = ex_fixup_addr(e);
> > +       handler = ex_fixup_handler(e);
> > +
> > +       if (!handler)
> > +               handler = ex_handler_default;
> 
> the !handler condition here will never trigger because the offset was
> already applied.

Actually, if I do "0 - .", that would overflow the int because current
location is virtual address and that's 64-bit. Or would gas simply
truncate it? Lemme check...

Anyway, what we should do instead is simply

	.long 0

to denote that the @handler is implicit.

Right?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302947

On Wed, Jan 6, 2016 at 9:59 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Jan 06, 2016 at 09:54:19AM -0800, Andy Lutomirski wrote:
>> I assume that this zero is to save the couple of bytes for the
>> relocation entry on relocatable kernels?
>
> I didn't want to touch all _ASM_EXTABLE() macro invocations by adding a
> third param @handler which is redundant as we know which it is.

I see.  You could shove the .long ex_handler_default - . into the
macro, but that would indeed bloat the kernel image a bit more
(although not the in-memory size of the kernel).

>
>> > +       new_ip  = ex_fixup_addr(e);
>> > +       handler = ex_fixup_handler(e);
>> > +
>> > +       if (!handler)
>> > +               handler = ex_handler_default;
>>
>> the !handler condition here will never trigger because the offset was
>> already applied.
>
> Actually, if I do "0 - .", that would overflow the int because current
> location is virtual address and that's 64-bit. Or would gas simply
> truncate it? Lemme check...
>
> Anyway, what we should do instead is simply
>
>         .long 0
>
> to denote that the @handler is implicit.
>
> Right?

Agreed.  I just think that your current fixup_ex_handler
implementation needs adjustment if you do it that way.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303024

On Wed, Jan 06, 2016 at 10:07:19AM -0800, Andy Lutomirski wrote:
> Agreed.  I just think that your current fixup_ex_handler
> implementation needs adjustment if you do it that way.

Right, and as you just mentioned on IRC, there's also sortextable.c
which needs adjusting. So I'll go stare at that code first to try to
figure out what exactly is being done there...

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303530

On Wed, Jan 06, 2016 at 08:42:22PM +0100, Borislav Petkov wrote:
> On Wed, Jan 06, 2016 at 10:07:19AM -0800, Andy Lutomirski wrote:
> > Agreed.  I just think that your current fixup_ex_handler
> > implementation needs adjustment if you do it that way.
> 
> Right, and as you just mentioned on IRC, there's also sortextable.c
> which needs adjusting. So I'll go stare at that code first to try to
> figure out what exactly is being done there...

Sorry, it took me a while to figure out all that normalization and
denormalization of the offsets sortextable.c does. And with the bigger
table of 3 ints, we need to do the sorting a bit differently. Also, the
alignment of 8 makes gas turn ".long 0" into 8 bytes worth of zeroes so
I had to do ".balign 4". Is that going to be a problem...?

With it, the exception table looks ok to me:

8e9984          35 ae 91 ff 73 d5 ff ff 00 00 00 00
8e9990          3e af 91 ff 71 d5 ff ff 00 00 00 00
8e999c          4e af 91 ff 6c d5 ff ff 00 00 00 00
8e99a8          49 af 91 ff 67 d5 ff ff 00 00 00 00
8e99b4          ab b3 91 ff 62 d5 ff ff 00 00 00 00
8e99c0          b0 b3 91 ff 5d d5 ff ff 00 00 00 00
8e99cc          e7 b3 91 ff 5b d5 ff ff 00 00 00 00
8e99d8          82 b4 91 ff 59 d5 ff ff 00 00 00 00
8e99e4          8e b9 91 ff 8e b9 91 ff d8 41 96 ff
8e99f0          8a b9 91 ff 8a b9 91 ff cc 41 96 ff
8e99fc          86 b9 91 ff 86 b9 91 ff c0 41 96 ff
8e9a08          82 b9 91 ff 82 b9 91 ff b4 41 96 ff
8e9a14          81 b9 91 ff 81 b9 91 ff a8 41 96 ff
8e9a20          7d b9 91 ff 7d b9 91 ff 9c 41 96 ff
...

the last int not 00 is the offset to ex_handler_ext(). Regarding that, I
probably should change _ASM_EXTABLE_EX() to take a @handler argument so
that Tony can pass in the ex_handler_fault() too.

Anyway, here's what I have, it boots fine in a guest.

Btw, it seems I'm coming down with the cold and all that above could be
hallucinations so please double-check me.

Thanks.

---
diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 189679aba703..20b638d49f24 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -46,16 +46,18 @@
 #ifdef __ASSEMBLY__
 # define _ASM_EXTABLE(from,to)					\
 	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
+	.balign 4 ;						\
 	.long (from) - . ;					\
 	.long (to) - . ;					\
+	.long 0;						\
 	.popsection
 
 # define _ASM_EXTABLE_EX(from,to)				\
 	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
+	.balign 4 ;						\
 	.long (from) - . ;					\
-	.long (to) - . + 0x7ffffff0 ;				\
+	.long (to) - . ;					\
+	.long ex_handler_ext - . ;				\
 	.popsection
 
 # define _ASM_NOKPROBE(entry)					\
@@ -91,16 +93,18 @@
 #else
 # define _ASM_EXTABLE(from,to)					\
 	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
+	" .balign 4\n"						\
 	" .long (" #from ") - .\n"				\
 	" .long (" #to ") - .\n"				\
+	" .long 0\n"						\
 	" .popsection\n"
 
 # define _ASM_EXTABLE_EX(from,to)				\
 	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
+	" .balign 4\n"						\
 	" .long (" #from ") - .\n"				\
-	" .long (" #to ") - . + 0x7ffffff0\n"			\
+	" .long (" #to ") - .\n"				\
+	" .long ex_handler_ext - .\n"				\
 	" .popsection\n"
 /* For C file, we already have NOKPROBE_SYMBOL macro */
 #endif
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 660458af425d..35d67026bfa5 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -104,13 +104,13 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
  */
 
 struct exception_table_entry {
-	int insn, fixup;
+	int insn, fixup, handler;
 };
 /* This is not the generic standard exception_table_entry format */
 #define ARCH_HAS_SORT_EXTABLE
 #define ARCH_HAS_SEARCH_EXTABLE
 
-extern int fixup_exception(struct pt_regs *regs);
+extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern int early_fixup_exception(unsigned long *ip);
 
 /*
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 1deffe6cc873..0f05deeff5ce 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -988,7 +988,7 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 		 * In case the user-specified fault handler returned
 		 * zero, try to fix up.
 		 */
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, trapnr))
 			return 1;
 
 		/*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index ade185a46b1d..211c11c7bba4 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -199,7 +199,7 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str,
 	}
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			tsk->thread.error_code = error_code;
 			tsk->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
@@ -453,7 +453,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
 
 	tsk = current;
 	if (!user_mode(regs)) {
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, X86_TRAP_GP))
 			return;
 
 		tsk->thread.error_code = error_code;
@@ -699,7 +699,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	conditional_sti(regs);
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			task->thread.error_code = error_code;
 			task->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 903ec1e9c326..e11d3b32efd4 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -3,6 +3,8 @@
 #include <linux/sort.h>
 #include <asm/uaccess.h>
 
+typedef int (*ex_handler_t)(const struct exception_table_entry *, struct pt_regs *, int);
+
 static inline unsigned long
 ex_insn_addr(const struct exception_table_entry *x)
 {
@@ -14,10 +16,42 @@ ex_fixup_addr(const struct exception_table_entry *x)
 	return (unsigned long)&x->fixup + x->fixup;
 }
 
-int fixup_exception(struct pt_regs *regs)
+int ex_handler_default(const struct exception_table_entry *fixup,
+		       struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+
+int ex_handler_fault(const struct exception_table_entry *fixup,
+		     struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+	regs->ax = trapnr;
+	return 1;
+}
+int ex_handler_ext(const struct exception_table_entry *fixup,
+		   struct pt_regs *regs, int trapnr)
+{
+	/* Special hack for uaccess_err */
+	current_thread_info()->uaccess_err = 1;
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+
+inline ex_handler_t ex_fixup_handler(const struct exception_table_entry *x)
+{
+	if (!x->handler)
+		return ex_handler_default;
+
+	return (ex_handler_t)&x->handler + x->handler;
+}
+
+int fixup_exception(struct pt_regs *regs, int trapnr)
 {
-	const struct exception_table_entry *fixup;
+	const struct exception_table_entry *e;
 	unsigned long new_ip;
+	ex_handler_t handler;
 
 #ifdef CONFIG_PNPBIOS
 	if (unlikely(SEGMENT_IS_PNP_CODE(regs->cs))) {
@@ -33,42 +67,36 @@ int fixup_exception(struct pt_regs *regs)
 	}
 #endif
 
-	fixup = search_exception_tables(regs->ip);
-	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
-
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* Special hack for uaccess_err */
-			current_thread_info()->uaccess_err = 1;
-			new_ip -= 0x7ffffff0;
-		}
-		regs->ip = new_ip;
-		return 1;
-	}
+	e = search_exception_tables(regs->ip);
+	if (!e)
+		return 0;
 
-	return 0;
+	new_ip  = ex_fixup_addr(e);
+	handler = ex_fixup_handler(e);
+
+	return handler(e, regs, trapnr);
 }
 
 /* Restricted version used during very early boot */
 int __init early_fixup_exception(unsigned long *ip)
 {
-	const struct exception_table_entry *fixup;
+	const struct exception_table_entry *e;
 	unsigned long new_ip;
+	ex_handler_t handler;
 
-	fixup = search_exception_tables(*ip);
-	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
+	e = search_exception_tables(*ip);
+	if (!e)
+		return 0;
 
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* uaccess handling not supported during early boot */
-			return 0;
-		}
+	new_ip  = ex_fixup_addr(e);
+	handler = ex_fixup_handler(e);
 
-		*ip = new_ip;
-		return 1;
-	}
+	/* uaccess handling not supported during early boot */
+	if (handler == ex_handler_ext)
+		return 0;
 
-	return 0;
+	*ip = new_ip;
+	return 1;
 }
 
 /*
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 546fbca9621d..def6af7aef89 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -670,7 +670,7 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 	int sig;
 
 	/* Are we prepared to handle this kernel fault? */
-	if (fixup_exception(regs)) {
+	if (fixup_exception(regs, X86_TRAP_PF)) {
 		/*
 		 * Any interrupt that takes a fault gets the fixup. This makes
 		 * the below recursive fault logic only apply to a faults from
diff --git a/scripts/sortextable.c b/scripts/sortextable.c
index c2423d913b46..b17b716959a4 100644
--- a/scripts/sortextable.c
+++ b/scripts/sortextable.c
@@ -209,6 +209,33 @@ static int compare_relative_table(const void *a, const void *b)
 	return 0;
 }
 
+static void x86_sort_relative_table(char *extab_image, int image_size)
+{
+	int i;
+
+	i = 0;
+	while (i < image_size) {
+		uint32_t *loc = (uint32_t *)(extab_image + i);
+
+		w(r(loc) + i, loc);
+		w(r(loc + 1) + i + 4, loc + 1);
+
+		i += sizeof(uint32_t) * 3;
+	}
+
+	qsort(extab_image, image_size / 12, 12, compare_relative_table);
+
+	i = 0;
+	while (i < image_size) {
+		uint32_t *loc = (uint32_t *)(extab_image + i);
+
+		w(r(loc) - i, loc);
+		w(r(loc + 1) - (i + 4), loc + 1);
+
+		i += sizeof(uint32_t) * 3;
+	}
+}
+
 static void sort_relative_table(char *extab_image, int image_size)
 {
 	int i;
@@ -281,6 +308,9 @@ do_file(char const *const fname)
 		break;
 	case EM_386:
 	case EM_X86_64:
+		custom_sort = x86_sort_relative_table;
+		break;
+
 	case EM_S390:
 		custom_sort = sort_relative_table;
 		break;


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303782

On Thu, Jan 07, 2016 at 01:11:31PM +0100, Borislav Petkov wrote:
>  #ifdef __ASSEMBLY__
>  # define _ASM_EXTABLE(from,to)					\
>  	.pushsection "__ex_table","a" ;				\
> -	.balign 8 ;						\
> +	.balign 4 ;						\
>  	.long (from) - . ;					\
>  	.long (to) - . ;					\
> +	.long 0;						\

Why not
	.long ex_handler_default - . ;

Then you wouldn't have to special case the zero in the lookup
(and in the sort, which you don't do now, but should)

> @@ -33,42 +67,36 @@ int fixup_exception(struct pt_regs *regs)
>  	}
>  #endif
>  
> -	fixup = search_exception_tables(regs->ip);
> -	if (fixup) {
> -		new_ip = ex_fixup_addr(fixup);
> -
> -		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
> -			/* Special hack for uaccess_err */
> -			current_thread_info()->uaccess_err = 1;
> -			new_ip -= 0x7ffffff0;
> -		}
> -		regs->ip = new_ip;
> -		return 1;
> -	}
> +	e = search_exception_tables(regs->ip);
> +	if (!e)
> +		return 0;
>  
> -	return 0;
> +	new_ip  = ex_fixup_addr(e);

Assigned, but not used - delete the declaration above too.

> +static void x86_sort_relative_table(char *extab_image, int image_size)
> +{
> +	int i;
> +
> +	i = 0;
> +	while (i < image_size) {
> +		uint32_t *loc = (uint32_t *)(extab_image + i);
> +
> +		w(r(loc) + i, loc);
> +		w(r(loc + 1) + i + 4, loc + 1);
Need to twiddle the 'handler' field too (unless it is 0). If you
give up on the magic zero and fill in the offset to ex_handler_default
then I *think* you need:
		w(r(loc + 2) + i + 8, loc + 2);
the special case *might* be:
		if (r(loc + 2))
			w(r(loc + 2) + i + 8, loc + 2);
> +
> +		i += sizeof(uint32_t) * 3;
> +	}
> +
> +	qsort(extab_image, image_size / 12, 12, compare_relative_table);
> +
> +	i = 0;
> +	while (i < image_size) {
> +		uint32_t *loc = (uint32_t *)(extab_image + i);
> +
> +		w(r(loc) - i, loc);
> +		w(r(loc + 1) - (i + 4), loc + 1);
ditto, untwiddle the handler (unless it was zero)
		w(r(loc + 2) - (i + 8), loc + 2);

There is also arch/x86/mm/extable.c:sort_extable() which will
be used on the main kernel exception table if for some reason
the build skipped using scripts/sortextable ... and is always
used for exception tables in modules.  It also needs to know
about the new field (and would be another place to special case
the zero fields for the default handler).

-Tony

[toc] | [prev] | [next] | [standalone]

#1304107

On Thu, Jan 07, 2016 at 01:11:31PM +0100, Borislav Petkov wrote:
> Anyway, here's what I have, it boots fine in a guest.
> 
> Btw, it seems I'm coming down with the cold and all that above could be
> hallucinations so please double-check me.

Hardly any hallucinations ... here's an update with the changes
I mentioned in earlier e-mail.  Boots on actual h/w.


diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 189679aba703..c286db8ddc58 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -46,16 +46,18 @@
 #ifdef __ASSEMBLY__
 # define _ASM_EXTABLE(from,to)					\
 	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
+	.balign 4 ;						\
 	.long (from) - . ;					\
 	.long (to) - . ;					\
+	.long ex_handler_default - . ;				\
 	.popsection
 
 # define _ASM_EXTABLE_EX(from,to)				\
 	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
+	.balign 4 ;						\
 	.long (from) - . ;					\
-	.long (to) - . + 0x7ffffff0 ;				\
+	.long (to) - . ;					\
+	.long ex_handler_ext - . ;				\
 	.popsection
 
 # define _ASM_NOKPROBE(entry)					\
@@ -91,16 +93,18 @@
 #else
 # define _ASM_EXTABLE(from,to)					\
 	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
+	" .balign 4\n"						\
 	" .long (" #from ") - .\n"				\
 	" .long (" #to ") - .\n"				\
+	" .long ex_handler_default - .\n"			\
 	" .popsection\n"
 
 # define _ASM_EXTABLE_EX(from,to)				\
 	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
+	" .balign 4\n"						\
 	" .long (" #from ") - .\n"				\
-	" .long (" #to ") - . + 0x7ffffff0\n"			\
+	" .long (" #to ") - .\n"				\
+	" .long ex_handler_ext - .\n"				\
 	" .popsection\n"
 /* For C file, we already have NOKPROBE_SYMBOL macro */
 #endif
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a8df874f3e88..b8f6f7545679 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -104,13 +104,13 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
  */
 
 struct exception_table_entry {
-	int insn, fixup;
+	int insn, fixup, handler;
 };
 /* This is not the generic standard exception_table_entry format */
 #define ARCH_HAS_SORT_EXTABLE
 #define ARCH_HAS_SEARCH_EXTABLE
 
-extern int fixup_exception(struct pt_regs *regs);
+extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern int early_fixup_exception(unsigned long *ip);
 
 /*
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 1deffe6cc873..0f05deeff5ce 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -988,7 +988,7 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 		 * In case the user-specified fault handler returned
 		 * zero, try to fix up.
 		 */
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, trapnr))
 			return 1;
 
 		/*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 346eec73f7db..7f50e8f4a075 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -199,7 +199,7 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str,
 	}
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			tsk->thread.error_code = error_code;
 			tsk->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
@@ -453,7 +453,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
 
 	tsk = current;
 	if (!user_mode(regs)) {
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, X86_TRAP_GP))
 			return;
 
 		tsk->thread.error_code = error_code;
@@ -699,7 +699,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	conditional_sti(regs);
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			task->thread.error_code = error_code;
 			task->thread.trap_nr = trapnr;
 			die(str, regs, error_code);
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 903ec1e9c326..01098ad010dd 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -3,6 +3,8 @@
 #include <linux/sort.h>
 #include <asm/uaccess.h>
 
+typedef int (*ex_handler_t)(const struct exception_table_entry *, struct pt_regs *, int);
+
 static inline unsigned long
 ex_insn_addr(const struct exception_table_entry *x)
 {
@@ -14,10 +16,39 @@ ex_fixup_addr(const struct exception_table_entry *x)
 	return (unsigned long)&x->fixup + x->fixup;
 }
 
-int fixup_exception(struct pt_regs *regs)
+int ex_handler_default(const struct exception_table_entry *fixup,
+		       struct pt_regs *regs, int trapnr)
 {
-	const struct exception_table_entry *fixup;
-	unsigned long new_ip;
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+EXPORT_SYMBOL(ex_handler_default);
+
+int ex_handler_fault(const struct exception_table_entry *fixup,
+		     struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+	regs->ax = trapnr;
+	return 1;
+}
+int ex_handler_ext(const struct exception_table_entry *fixup,
+		   struct pt_regs *regs, int trapnr)
+{
+	/* Special hack for uaccess_err */
+	current_thread_info()->uaccess_err = 1;
+	regs->ip = ex_fixup_addr(fixup);
+	return 1;
+}
+
+inline ex_handler_t ex_fixup_handler(const struct exception_table_entry *x)
+{
+	return (ex_handler_t)&x->handler + x->handler;
+}
+
+int fixup_exception(struct pt_regs *regs, int trapnr)
+{
+	const struct exception_table_entry *e;
+	ex_handler_t handler;
 
 #ifdef CONFIG_PNPBIOS
 	if (unlikely(SEGMENT_IS_PNP_CODE(regs->cs))) {
@@ -33,42 +64,35 @@ int fixup_exception(struct pt_regs *regs)
 	}
 #endif
 
-	fixup = search_exception_tables(regs->ip);
-	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
-
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* Special hack for uaccess_err */
-			current_thread_info()->uaccess_err = 1;
-			new_ip -= 0x7ffffff0;
-		}
-		regs->ip = new_ip;
-		return 1;
-	}
+	e = search_exception_tables(regs->ip);
+	if (!e)
+		return 0;
 
-	return 0;
+	handler = ex_fixup_handler(e);
+
+	return handler(e, regs, trapnr);
 }
 
 /* Restricted version used during very early boot */
 int __init early_fixup_exception(unsigned long *ip)
 {
-	const struct exception_table_entry *fixup;
+	const struct exception_table_entry *e;
 	unsigned long new_ip;
+	ex_handler_t handler;
 
-	fixup = search_exception_tables(*ip);
-	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
+	e = search_exception_tables(*ip);
+	if (!e)
+		return 0;
 
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-			/* uaccess handling not supported during early boot */
-			return 0;
-		}
+	new_ip  = ex_fixup_addr(e);
+	handler = ex_fixup_handler(e);
 
-		*ip = new_ip;
-		return 1;
-	}
+	/* uaccess handling not supported during early boot */
+	if (handler == ex_handler_ext)
+		return 0;
 
-	return 0;
+	*ip = new_ip;
+	return 1;
 }
 
 /*
@@ -133,6 +157,8 @@ void sort_extable(struct exception_table_entry *start,
 		i += 4;
 		p->fixup += i;
 		i += 4;
+		p->handler += i;
+		i += 4;
 	}
 
 	sort(start, finish - start, sizeof(struct exception_table_entry),
@@ -145,6 +171,8 @@ void sort_extable(struct exception_table_entry *start,
 		i += 4;
 		p->fixup -= i;
 		i += 4;
+		p->handler -= i;
+		i += 4;
 	}
 }
 
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index eef44d9a3f77..495946c3f9dd 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -656,7 +656,7 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 	int sig;
 
 	/* Are we prepared to handle this kernel fault? */
-	if (fixup_exception(regs)) {
+	if (fixup_exception(regs, X86_TRAP_PF)) {
 		/*
 		 * Any interrupt that takes a fault gets the fixup. This makes
 		 * the below recursive fault logic only apply to a faults from
diff --git a/scripts/sortextable.c b/scripts/sortextable.c
index c2423d913b46..7b29fb14f870 100644
--- a/scripts/sortextable.c
+++ b/scripts/sortextable.c
@@ -209,6 +209,35 @@ static int compare_relative_table(const void *a, const void *b)
 	return 0;
 }
 
+static void x86_sort_relative_table(char *extab_image, int image_size)
+{
+	int i;
+
+	i = 0;
+	while (i < image_size) {
+		uint32_t *loc = (uint32_t *)(extab_image + i);
+
+		w(r(loc) + i, loc);
+		w(r(loc + 1) + i + 4, loc + 1);
+		w(r(loc + 2) + i + 8, loc + 2);
+
+		i += sizeof(uint32_t) * 3;
+	}
+
+	qsort(extab_image, image_size / 12, 12, compare_relative_table);
+
+	i = 0;
+	while (i < image_size) {
+		uint32_t *loc = (uint32_t *)(extab_image + i);
+
+		w(r(loc) - i, loc);
+		w(r(loc + 1) - (i + 4), loc + 1);
+		w(r(loc + 2) - (i + 8), loc + 2);
+
+		i += sizeof(uint32_t) * 3;
+	}
+}
+
 static void sort_relative_table(char *extab_image, int image_size)
 {
 	int i;
@@ -281,6 +310,9 @@ do_file(char const *const fname)
 		break;
 	case EM_386:
 	case EM_X86_64:
+		custom_sort = x86_sort_relative_table;
+		break;
+
 	case EM_S390:
 		custom_sort = sort_relative_table;
 		break;

[toc] | [prev] | [next] | [standalone]

#1304355

On Thu, Jan 07, 2016 at 05:45:26PM -0800, Luck, Tony wrote:
> On Thu, Jan 07, 2016 at 01:11:31PM +0100, Borislav Petkov wrote:
> > Anyway, here's what I have, it boots fine in a guest.
> > 
> > Btw, it seems I'm coming down with the cold and all that above could be
> > hallucinations so please double-check me.
> 
> Hardly any hallucinations ... here's an update with the changes
> I mentioned in earlier e-mail.  Boots on actual h/w.

Cool, thanks for fixing it up. Looks good to me. Feel free to make it
into a proper patch and add it to your series... unless you want me to
do it.

Just one small question below:

> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 903ec1e9c326..01098ad010dd 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -3,6 +3,8 @@
>  #include <linux/sort.h>
>  #include <asm/uaccess.h>
>  
> +typedef int (*ex_handler_t)(const struct exception_table_entry *, struct pt_regs *, int);
> +
>  static inline unsigned long
>  ex_insn_addr(const struct exception_table_entry *x)
>  {
> @@ -14,10 +16,39 @@ ex_fixup_addr(const struct exception_table_entry *x)
>  	return (unsigned long)&x->fixup + x->fixup;
>  }
>  
> -int fixup_exception(struct pt_regs *regs)
> +int ex_handler_default(const struct exception_table_entry *fixup,
> +		       struct pt_regs *regs, int trapnr)
>  {
> -	const struct exception_table_entry *fixup;
> -	unsigned long new_ip;
> +	regs->ip = ex_fixup_addr(fixup);
> +	return 1;
> +}
> +EXPORT_SYMBOL(ex_handler_default);

Why not EXPORT_SYMBOL_GPL() ?

We do not care about external modules.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1304707

>> +EXPORT_SYMBOL(ex_handler_default);
>
> Why not EXPORT_SYMBOL_GPL() ?
>
> We do not care about external modules.

I thought the guideline was that new features are GPL, but changes
to existing features shouldn't break by adding new GPL requirements.

The point is moot though because  the shared hallucinations wore
off this morning and I realized that having the "handler" be a pointer
to a function can't work. We're storing the 32-bit signed offset from
the extable to the target address. This is fine if the table and the
address are close together. But for modules we have an exception
table wherever vmalloc() loaded the module, and a function back
in the base kernel.

So back to your ".long 0" for the default case.  And if we want to allow
modules to use any of the new handlers, then we can't use
relative function pointers for them either.

So I'm looking at making the new field just a simple integer and using
it to index an array of function pointers (like in v7).

Unless someone has a better idea?

-Tony

[toc] | [prev] | [next] | [standalone]

#1304749

On Fri, Jan 08, 2016 at 04:29:49PM +0000, Luck, Tony wrote:
> I thought the guideline was that new features are GPL, but changes
> to existing features shouldn't break by adding new GPL requirements.
> 
> The point is moot though because  the shared hallucinations wore
> off this morning and I realized that having the "handler" be a pointer
> to a function can't work. We're storing the 32-bit signed offset from
> the extable to the target address. This is fine if the table and the
> address are close together. But for modules we have an exception
> table wherever vmalloc() loaded the module, and a function back
> in the base kernel.

Whoops, true story.

> So back to your ".long 0" for the default case.  And if we want to allow
> modules to use any of the new handlers, then we can't use
> relative function pointers for them either.
> 
> So I'm looking at making the new field just a simple integer and using
> it to index an array of function pointers (like in v7).

Right, that sounds good too. I guess we can even split the integer into

[0 ... 7][8 ... 31]

where slice [0:7] is an index into the handlers array and the remaining
unused 24-bits could be used for other stuff later. Normal addition as a
way to OR values should work.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1305033

On Fri, Jan 8, 2016 at 11:29 AM, Luck, Tony <tony.luck@intel.com> wrote:
>>> +EXPORT_SYMBOL(ex_handler_default);
>>
>> Why not EXPORT_SYMBOL_GPL() ?
>>
>> We do not care about external modules.
>
> I thought the guideline was that new features are GPL, but changes
> to existing features shouldn't break by adding new GPL requirements.
>
> The point is moot though because  the shared hallucinations wore
> off this morning and I realized that having the "handler" be a pointer
> to a function can't work. We're storing the 32-bit signed offset from
> the extable to the target address. This is fine if the table and the
> address are close together. But for modules we have an exception
> table wherever vmalloc() loaded the module, and a function back
> in the base kernel.
>
> So back to your ".long 0" for the default case.  And if we want to allow
> modules to use any of the new handlers, then we can't use
> relative function pointers for them either.
>
> So I'm looking at making the new field just a simple integer and using
> it to index an array of function pointers (like in v7).
>
> Unless someone has a better idea?

Aren't modules loaded in the top 2GB of address space like the main
kernel?  Otherwise rip-relative addressing wouldn't work and modules
would have to be compiled as PIC.

--
Brian Gerst

[toc] | [prev] | [next] | [standalone]

#1304193

Also need some comment and Documentation/ changes:


diff --git a/Documentation/x86/exception-tables.txt b/Documentation/x86/exception-tables.txt
index 32901aa36f0a..ae47b9f64b8a 100644
--- a/Documentation/x86/exception-tables.txt
+++ b/Documentation/x86/exception-tables.txt
@@ -290,3 +290,37 @@ Due to the way that the exception table is built and needs to be ordered,
 only use exceptions for code in the .text section.  Any other section
 will cause the exception table to not be sorted correctly, and the
 exceptions will fail.
+
+Things changed when 64-bit support was added to x86 Linux. Rather than
+double the size of the exception table by expanding the two entries
+from 32-bits to 64 bits, a clever trick was used to store addreesses
+as relative offsets from the table itself. The assembly code changed
+from:
+	.long 1b,3b
+to:
+        .long (from) - .
+        .long (to) - .
+and the C-code that uses these values converts back to absolute addresses
+like this:
+	ex_insn_addr(const struct exception_table_entry *x)
+	{
+		return (unsigned long)&x->insn + x->insn;
+	}
+
+In v4.5 the exception table entry was given a new field "handler".
+This is also 32-bits wide and contains a table entry relative address
+of a handler function that can perform specific operations in addition
+to re-writing the instruction pointer to jump to the fixup location.
+Initially there are three such functions:
+
+1) int ex_handler_default(const struct exception_table_entry *fixup,
+   This is legacy case that just jumps to the fixup code
+2) int ex_handler_fault(const struct exception_table_entry *fixup,
+   This case provides the fault number of the trap that occured at
+   entry->insn. It is used to distinguish page faults from machine
+   check.
+3) int ex_handler_ext(const struct exception_table_entry *fixup,
+   This case is used to for uaccess_err ... we need to set a flag
+   in the task structure. Before the handler functions existed this
+   case was handled by adding a large offset to the fixup to tag
+   it as special.
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index b8f6f7545679..563443870915 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -90,12 +90,12 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
 	likely(!__range_not_ok(addr, size, user_addr_max()))
 
 /*
- * The exception table consists of pairs of addresses relative to the
+ * The exception table consists of triples of addresses relative to the
  * exception table enty itself: the first is the address of an
- * instruction that is allowed to fault, and the second is the address
- * at which the program should continue.  No registers are modified,
- * so it is entirely up to the continuation code to figure out what to
- * do.
+ * instruction that is allowed to fault, the second is the address
+ * at which the program should continue, the last is the address of
+ * a handler function to deal with the fault referenced by the instruction
+ * in the first field.
  *
  * All the routines below use bits of fixup code that are out of line
  * with the main instruction path.  This means when everything is well,
diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index f057718d8d15..195ff0144152 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -310,4 +310,3 @@ ENTRY(__mcsafe_copy)
 	_ASM_EXTABLE_FAULT(12b,38b)
 	_ASM_EXTABLE_FAULT(18b,39b)
 	_ASM_EXTABLE_FAULT(21b,40b)
-#endif

[toc] | [prev] | [next] | [standalone]

#1304364

On Thu, Jan 07, 2016 at 09:30:29PM -0800, Luck, Tony wrote:
> Also need some comment and Documentation/ changes:
> 
> 
> diff --git a/Documentation/x86/exception-tables.txt b/Documentation/x86/exception-tables.txt
> index 32901aa36f0a..ae47b9f64b8a 100644
> --- a/Documentation/x86/exception-tables.txt
> +++ b/Documentation/x86/exception-tables.txt
> @@ -290,3 +290,37 @@ Due to the way that the exception table is built and needs to be ordered,
>  only use exceptions for code in the .text section.  Any other section
>  will cause the exception table to not be sorted correctly, and the
>  exceptions will fail.
> +
> +Things changed when 64-bit support was added to x86 Linux. Rather than
> +double the size of the exception table by expanding the two entries
> +from 32-bits to 64 bits, a clever trick was used to store addreesses
> +as relative offsets from the table itself. The assembly code changed
> +from:
> +	.long 1b,3b
> +to:
> +        .long (from) - .
> +        .long (to) - .
> +and the C-code that uses these values converts back to absolute addresses
> +like this:
> +	ex_insn_addr(const struct exception_table_entry *x)
> +	{
> +		return (unsigned long)&x->insn + x->insn;
> +	}
> +
> +In v4.5 the exception table entry was given a new field "handler".
> +This is also 32-bits wide and contains a table entry relative address
> +of a handler function that can perform specific operations in addition
> +to re-writing the instruction pointer to jump to the fixup location.
> +Initially there are three such functions:
> +
> +1) int ex_handler_default(const struct exception_table_entry *fixup,
> +   This is legacy case that just jumps to the fixup code
> +2) int ex_handler_fault(const struct exception_table_entry *fixup,
> +   This case provides the fault number of the trap that occured at
> +   entry->insn. It is used to distinguish page faults from machine
> +   check.
> +3) int ex_handler_ext(const struct exception_table_entry *fixup,
> +   This case is used to for uaccess_err ... we need to set a flag
> +   in the task structure. Before the handler functions existed this
> +   case was handled by adding a large offset to the fixup to tag
> +   it as special.
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index b8f6f7545679..563443870915 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -90,12 +90,12 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
>  	likely(!__range_not_ok(addr, size, user_addr_max()))
>  
>  /*
> - * The exception table consists of pairs of addresses relative to the
> + * The exception table consists of triples of addresses relative to the
>   * exception table enty itself: the first is the address of an
> - * instruction that is allowed to fault, and the second is the address
> - * at which the program should continue.  No registers are modified,
> - * so it is entirely up to the continuation code to figure out what to
> - * do.
> + * instruction that is allowed to fault, the second is the address
> + * at which the program should continue, the last is the address of
> + * a handler function to deal with the fault referenced by the instruction
> + * in the first field.
>   *
>   * All the routines below use bits of fixup code that are out of line
>   * with the main instruction path.  This means when everything is well,

Looks good. /me always likes patches adding more sensible documentation:

Acked-by: Borislav Petkov <bp@suse.de>

> diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
> index f057718d8d15..195ff0144152 100644
> --- a/arch/x86/lib/memcpy_64.S
> +++ b/arch/x86/lib/memcpy_64.S
> @@ -310,4 +310,3 @@ ENTRY(__mcsafe_copy)
>  	_ASM_EXTABLE_FAULT(12b,38b)
>  	_ASM_EXTABLE_FAULT(18b,39b)
>  	_ASM_EXTABLE_FAULT(21b,40b)
> -#endif

This looks like a stray change.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1302724

On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
> Starting with a patch from Andy Lutomirski <luto@amacapital.net>
> that used linker relocation trickery to free up a couple of bits
> in the "fixup" field of the exception table (and generalized the
> uaccess_err hack to use one of the classes).
> 
> This patch allocates another one of the classes to provide
> a mechanism to provide the fault number to the fixup code
> in %rax.
> 
> Still one free class for the next brilliant idea. If more are
> needed it should be possible to squeeze another bit or three
> extending the same technique.
> 
> Originally-from: Andy Lutomirski <luto@amacapital.net>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/include/asm/asm.h     | 102 +++++++++++++++++++++++++++++++----------
>  arch/x86/include/asm/uaccess.h |  17 +++++--
>  arch/x86/kernel/kprobes/core.c |   2 +-
>  arch/x86/kernel/traps.c        |   6 +--
>  arch/x86/mm/extable.c          |  66 ++++++++++++++++++--------
>  arch/x86/mm/fault.c            |   2 +-
>  6 files changed, 142 insertions(+), 53 deletions(-)

...

> @@ -699,7 +699,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
>  	conditional_sti(regs);
>  
>  	if (!user_mode(regs)) {
> -		if (!fixup_exception(regs)) {
> +		if (!fixup_exception(regs, X86_TRAP_DE)) {

Whatever we end up doing, this needs to be trapnr above and not X86_TRAP_DE.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web