[PATCH] x86/extable: ensure entries are swapped completely when sorting

[PATCH] x86/extable: ensure entries are swapped completely when sorting

[Mathias Krause]

The x86 exception table sorting was changed in commit 29934b0fb8ff
("x86/extable: use generic search and sort routines") to use the arch
independent code in lib/extable.c. However, the patch was mangled
somehow on its way into the kernel from the last version posted at [1].
The committed version kind of attempted to incorporate the changes of
commit 548acf19234d ("x86/mm: Expand the exception table logic to allow
new handling options") as in _completely_ _ignoring_ the x86 specific
'handler' member of struct exception_table_entry. This effectively broke
the sorting as entries will only partly be swapped now.

Fortunately, the x86 Kconfig selects BUILDTIME_EXTABLE_SORT, so the
exception table doesn't need to be sorted at runtime. However, in case
that ever changes, we better not break the exception table sorting just
because of that.

Fix this by providing a swap_ex_entry_fixup() macro that takes care of
the 'handler' member.

[1] https://lkml.org/lkml/2016/1/27/232

Signed-off-by: Mathias Krause <minipli@googlemail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: H. Peter Anvin <hpa@linux.intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/uaccess.h |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a969ae607be8..b69b7bffb0e0 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -108,6 +108,14 @@ struct exception_table_entry {
 
 #define ARCH_HAS_RELATIVE_EXTABLE
 
+#define swap_ex_entry_fixup(a, b, tmp, delta)			\
+	do {							\
+		(a)->fixup = (b)->fixup + (delta);		\
+		(b)->fixup = (tmp).fixup - (delta);		\
+		(a)->handler = (b)->handler + (delta);		\
+		(b)->handler = (tmp).handler - (delta);		\
+	} while (0)
+
 extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern bool ex_has_fault_handler(unsigned long ip);
 extern int early_fixup_exception(unsigned long *ip);
-- 
1.7.10.4

Re: [PATCH] x86/extable: ensure entries are swapped completely when sorting

[Ard Biesheuvel]

On 10 May 2016 at 23:07, Mathias Krause <minipli@googlemail.com> wrote:
> The x86 exception table sorting was changed in commit 29934b0fb8ff
> ("x86/extable: use generic search and sort routines") to use the arch
> independent code in lib/extable.c. However, the patch was mangled
> somehow on its way into the kernel from the last version posted at [1].
> The committed version kind of attempted to incorporate the changes of
> commit 548acf19234d ("x86/mm: Expand the exception table logic to allow
> new handling options") as in _completely_ _ignoring_ the x86 specific
> 'handler' member of struct exception_table_entry. This effectively broke
> the sorting as entries will only partly be swapped now.
>

OK, it appears that Tony's patches grew a 'handler' field fairly late
in their review cycle, and I didn't pick up on that. Apologies.

> Fortunately, the x86 Kconfig selects BUILDTIME_EXTABLE_SORT, so the
> exception table doesn't need to be sorted at runtime. However, in case
> that ever changes, we better not break the exception table sorting just
> because of that.
>

BUILDTIME_EXTABLE_SORT applies to the core image only, but we still
rely on the sorting routines for modules in that case. So I think this
needs to be fixed right away.

> Fix this by providing a swap_ex_entry_fixup() macro that takes care of
> the 'handler' member.
>
> [1] https://lkml.org/lkml/2016/1/27/232
>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: H. Peter Anvin <hpa@linux.intel.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>

Fixes: 29934b0fb8f ("x86/extable: use generic search and sort routines")
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks,
Ard.

> ---
>  arch/x86/include/asm/uaccess.h |    8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index a969ae607be8..b69b7bffb0e0 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -108,6 +108,14 @@ struct exception_table_entry {
>
>  #define ARCH_HAS_RELATIVE_EXTABLE
>
> +#define swap_ex_entry_fixup(a, b, tmp, delta)                  \
> +       do {                                                    \
> +               (a)->fixup = (b)->fixup + (delta);              \
> +               (b)->fixup = (tmp).fixup - (delta);             \
> +               (a)->handler = (b)->handler + (delta);          \
> +               (b)->handler = (tmp).handler - (delta);         \
> +       } while (0)
> +
>  extern int fixup_exception(struct pt_regs *regs, int trapnr);
>  extern bool ex_has_fault_handler(unsigned long ip);
>  extern int early_fixup_exception(unsigned long *ip);
> --
> 1.7.10.4
>

Re: [PATCH] x86/extable: ensure entries are swapped completely when sorting

[Mathias Krause]

On 11 May 2016 at 07:46, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 10 May 2016 at 23:07, Mathias Krause <minipli@googlemail.com> wrote:
>> The x86 exception table sorting was changed in commit 29934b0fb8ff
>> ("x86/extable: use generic search and sort routines") to use the arch
>> independent code in lib/extable.c. However, the patch was mangled
>> somehow on its way into the kernel from the last version posted at [1].
>> The committed version kind of attempted to incorporate the changes of
>> commit 548acf19234d ("x86/mm: Expand the exception table logic to allow
>> new handling options") as in _completely_ _ignoring_ the x86 specific
>> 'handler' member of struct exception_table_entry. This effectively broke
>> the sorting as entries will only partly be swapped now.
>>
>
> OK, it appears that Tony's patches grew a 'handler' field fairly late
> in their review cycle, and I didn't pick up on that. Apologies.

No need to apologize, as your patch *must* have created conflicts for
the committer applying it. It's just that those conflicts were
resolved wrongly. He/She should have asked you for a new version,
but... did not :/

>> Fortunately, the x86 Kconfig selects BUILDTIME_EXTABLE_SORT, so the
>> exception table doesn't need to be sorted at runtime. However, in case
>> that ever changes, we better not break the exception table sorting just
>> because of that.
>>
>
> BUILDTIME_EXTABLE_SORT applies to the core image only, but we still
> rely on the sorting routines for modules in that case. So I think this
> needs to be fixed right away.

Good point!

>
>> Fix this by providing a swap_ex_entry_fixup() macro that takes care of
>> the 'handler' member.
>>
>> [1] https://lkml.org/lkml/2016/1/27/232
>>
>> Signed-off-by: Mathias Krause <minipli@googlemail.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: H. Peter Anvin <hpa@linux.intel.com>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Tony Luck <tony.luck@intel.com>
>
> Fixes: 29934b0fb8f ("x86/extable: use generic search and sort routines")
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Thanks,
> Ard.
>

Thanks,
Mathias

[tip:x86/mm] x86/extable: Ensure entries are swapped completely when sorting

[tip-bot for Mathias Krause]

Commit-ID:  67d7a982bab6702d84415ea889996fae72a7d3b2
Gitweb:     http://git.kernel.org/tip/67d7a982bab6702d84415ea889996fae72a7d3b2
Author:     Mathias Krause <minipli@googlemail.com>
AuthorDate: Tue, 10 May 2016 23:07:02 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 11 May 2016 11:14:06 +0200

x86/extable: Ensure entries are swapped completely when sorting

The x86 exception table sorting was changed in this recent commit:

  29934b0fb8ff ("x86/extable: use generic search and sort routines")

... to use the arch independent code in lib/extable.c. However, the
patch was mangled somehow on its way into the kernel from the last
version posted at:

  https://lkml.org/lkml/2016/1/27/232

The committed version kind of attempted to incorporate the changes of
contemporary commit done in the x86 tree:

  548acf19234d ("x86/mm: Expand the exception table logic to allow new handling options")

... as in _completely_ _ignoring_ the x86 specific 'handler' member of
struct exception_table_entry. This effectively broke the sorting as
entries will only be partly swapped now.

Fortunately, the x86 Kconfig selects BUILDTIME_EXTABLE_SORT, so the
exception table doesn't need to be sorted at runtime. However, in case
that ever changes, we better not break the exception table sorting just
because of that.

Fix this by providing a swap_ex_entry_fixup() macro that takes care of
the 'handler' member.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
Link: http://lkml.kernel.org/r/1462914422-2911-1-git-send-email-minipli@googlemail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/uaccess.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 8b3fb76..86c48f3 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -108,6 +108,14 @@ struct exception_table_entry {
 
 #define ARCH_HAS_RELATIVE_EXTABLE
 
+#define swap_ex_entry_fixup(a, b, tmp, delta)			\
+	do {							\
+		(a)->fixup = (b)->fixup + (delta);		\
+		(b)->fixup = (tmp).fixup - (delta);		\
+		(a)->handler = (b)->handler + (delta);		\
+		(b)->handler = (tmp).handler - (delta);		\
+	} while (0)
+
 extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern bool ex_has_fault_handler(unsigned long ip);
 extern int early_fixup_exception(unsigned long *ip);