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

2016-01-07 Thread Borislav Petkov 
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) - . + 0x7ff0 ;   \
+   .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 ") - . + 0x7ff0\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

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

2016-01-07 Thread Borislav Petkov 
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) - . + 0x7ff0 ;   \
+   .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 ") - . + 0x7ff0\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

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

2016-01-06 Thread Borislav Petkov 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Andy Lutomirski 
On Wed, Jan 6, 2016 at 9:59 AM, Borislav Petkov  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Borislav Petkov 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Andy Lutomirski 
On Wed, Jan 6, 2016 at 4:33 AM, Borislav Petkov  wrote:
> On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
>> Starting with a patch from Andy Lutomirski 
>> 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)>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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Linus Torvalds 
On Wed, Jan 6, 2016 at 9:35 AM, Luck, Tony  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Luck, Tony 
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 
> > 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 0008  
 02a8 2**3
arch/x86/kvm/kvm-intel.ko 7 __ex_table 09b8  
 0001ff60 2**3
arch/x86/kvm/kvm-amd.ko 6 __ex_table 0070   
a1e0 2**3
arch/x86/kvm/kvm.ko 23 __ex_table 0080   
00059940 2**3
drivers/char/ipmi/ipmi_devintf.ko 9 __ex_table 0098  
 17f0 2**3
drivers/gpu/drm/i915/i915.ko 20 __ex_table 0060  
 000f8f10 2**3
drivers/gpu/drm/radeon/radeon.ko 19 __ex_table 01d0  
 00145c88 2**3
drivers/gpu/drm/drm.ko 24 __ex_table 0400   
0003b688 2**3
drivers/media/usb/uvc/uvcvideo.ko 15 __ex_table 0030  
 f100 2**3
drivers/scsi/sg.ko 12 __ex_table 0060   
6548 2**3
drivers/vhost/vhost.ko 12 __ex_table 0068   
3360 2**3
drivers/xen/xen-privcmd.ko 11 __ex_table 0010  
 0f70 2**3
net/sunrpc/sunrpc.ko 24 __ex_table 0008   
000321b0 2**3
sound/core/snd-pcm.ko 19 __ex_table 0040   
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) - . + 0x7ff0 ;   \
> + .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 ") - . + 0x7ff0\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,

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

2016-01-06 Thread Borislav Petkov 
On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
> Starting with a patch from Andy Lutomirski 
> 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 
> Signed-off-by: Tony Luck 
> ---
>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Borislav Petkov 
On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
> Starting with a patch from Andy Lutomirski 
> 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) - . + 0x7ff0 ;   \
+   .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 ") - . + 0x7ff0\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

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

2016-01-06 Thread Borislav Petkov 
On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
> Starting with a patch from Andy Lutomirski 
> 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 
> Signed-off-by: Tony Luck 
> ---
>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Borislav Petkov 
On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
> Starting with a patch from Andy Lutomirski 
> 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) - . + 0x7ff0 ;   \
+   .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 ") - . + 0x7ff0\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)) {
-

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

2016-01-06 Thread Linus Torvalds 
On Wed, Jan 6, 2016 at 9:35 AM, Luck, Tony  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Borislav Petkov 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Andy Lutomirski 
On Wed, Jan 6, 2016 at 4:33 AM, Borislav Petkov  wrote:
> On Wed, Dec 30, 2015 at 09:59:29AM -0800, Tony Luck wrote:
>> Starting with a patch from Andy Lutomirski 
>> 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)>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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Andy Lutomirski 
On Wed, Jan 6, 2016 at 9:59 AM, Borislav Petkov  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2016-01-06 Thread Luck, Tony 
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 
> > 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 0008  
 02a8 2**3
arch/x86/kvm/kvm-intel.ko 7 __ex_table 09b8  
 0001ff60 2**3
arch/x86/kvm/kvm-amd.ko 6 __ex_table 0070   
a1e0 2**3
arch/x86/kvm/kvm.ko 23 __ex_table 0080   
00059940 2**3
drivers/char/ipmi/ipmi_devintf.ko 9 __ex_table 0098  
 17f0 2**3
drivers/gpu/drm/i915/i915.ko 20 __ex_table 0060  
 000f8f10 2**3
drivers/gpu/drm/radeon/radeon.ko 19 __ex_table 01d0  
 00145c88 2**3
drivers/gpu/drm/drm.ko 24 __ex_table 0400   
0003b688 2**3
drivers/media/usb/uvc/uvcvideo.ko 15 __ex_table 0030  
 f100 2**3
drivers/scsi/sg.ko 12 __ex_table 0060   
6548 2**3
drivers/vhost/vhost.ko 12 __ex_table 0068   
3360 2**3
drivers/xen/xen-privcmd.ko 11 __ex_table 0010  
 0f70 2**3
net/sunrpc/sunrpc.ko 24 __ex_table 0008   
000321b0 2**3
sound/core/snd-pcm.ko 19 __ex_table 0040   
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) - . + 0x7ff0 ;   \
> + .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 ") - . + 0x7ff0\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

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

2016-01-06 Thread Borislav Petkov 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/