* [PATCH] x86/alternatives: Fix int3_emulate_call() selftest stack corruption
@ 2019-07-08 20:55 Josh Poimboeuf
2019-07-09 12:57 ` Peter Zijlstra
0 siblings, 1 reply; 4+ messages in thread
From: Josh Poimboeuf @ 2019-07-08 20:55 UTC (permalink / raw)
To: x86
Cc: linux-kernel, Peter Zijlstra, Andy Lutomirski, Ingo Molnar,
Thomas Gleixner, Linus Torvalds, kernel test robot
KASAN shows the following splat during boot:
BUG: KASAN: unknown-crash in unwind_next_frame+0x3f6/0x490
Read of size 8 at addr ffffffff84007db0 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Tainted: G T 5.2.0-rc6-00013-g7457c0d #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
dump_stack+0x19/0x1b
print_address_description+0x1b0/0x2b2
? unwind_next_frame+0x3f6/0x490
__kasan_report+0x10f/0x171
? unwind_next_frame+0x3f6/0x490
kasan_report+0x12/0x1c
__asan_load8+0x54/0x81
unwind_next_frame+0x3f6/0x490
? unwind_dump+0x24e/0x24e
unwind_next_frame+0x1b/0x23
? create_prof_cpu_mask+0x20/0x20
arch_stack_walk+0x68/0xa5
? set_memory_4k+0x2a/0x2c
stack_trace_save+0x7b/0xa0
? stack_trace_consume_entry+0x89/0x89
save_trace+0x3c/0x93
mark_lock+0x1ef/0x9b1
? sched_clock_local+0x86/0xa6
__lock_acquire+0x3ba/0x1bea
? __asan_loadN+0xf/0x11
? mark_held_locks+0x8e/0x8e
? mark_lock+0xb4/0x9b1
? sched_clock_local+0x86/0xa6
lock_acquire+0x122/0x221
? _vm_unmap_aliases+0x141/0x183
__mutex_lock+0xb6/0x731
? _vm_unmap_aliases+0x141/0x183
? sched_clock_cpu+0xac/0xb1
? __mutex_add_waiter+0xae/0xae
? lock_downgrade+0x368/0x368
? _vm_unmap_aliases+0x40/0x183
mutex_lock_nested+0x16/0x18
_vm_unmap_aliases+0x141/0x183
? _vm_unmap_aliases+0x40/0x183
vm_unmap_aliases+0x14/0x16
change_page_attr_set_clr+0x15e/0x2f2
? __set_pages_p+0x111/0x111
? alternative_instructions+0xd8/0x118
? arch_init_ideal_nops+0x181/0x181
set_memory_4k+0x2a/0x2c
check_bugs+0x11fd/0x1298
? l1tf_cmdline+0x1dc/0x1dc
? proc_create_single_data+0x5f/0x6e
? cgroup_init+0x2b1/0x2f6
start_kernel+0x793/0x7eb
? thread_stack_cache_init+0x2e/0x2e
? idt_setup_early_handler+0x70/0xb1
x86_64_start_reservations+0x55/0x76
x86_64_start_kernel+0x87/0xaa
secondary_startup_64+0xa4/0xb0
Memory state around the buggy address:
ffffffff84007c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1
ffffffff84007d00: f1 00 00 00 00 00 00 00 00 00 f2 f2 f2 f3 f3 f3
>ffffffff84007d80: f3 79 be 52 49 79 be 00 00 00 00 00 00 00 00 f1
It turns out that int3_selftest() is corrupting the stack. The problem
is that the KASAN-ified version of int3_magic() is much less trivial
than the C code appears. It clobbers several unexpected registers. So
when the selftest's INT3 is converted to an emulated call to
int3_magic(), the registers are clobbered and Bad Things happen when the
function returns.
Fix it by adding all the caller-saved registers to the inline asm
clobbers list.
Reported-by: kernel test robot <rong.a.chen@intel.com>
Fixes: 7457c0da024b ("x86/alternatives: Add int3_emulate_call() selftest")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/kernel/alternative.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 99ef8b6f9a1a..2644a7b82f96 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -651,6 +651,13 @@ int3_exception_notify(struct notifier_block *self, unsigned long val, void *data
return NOTIFY_STOP;
}
+#ifdef CONFIG_X86_32
+# define INT3_TEST_CLOBBERS "memory", "cc", "ecx", "edx"
+#else
+# define INT3_TEST_CLOBBERS "memory", "cc", "rax", "rcx", "rdx", "rsi", "r8", \
+ "r9", "r10", "r11"
+#endif
+
static void __init int3_selftest(void)
{
static __initdata struct notifier_block int3_exception_nb = {
@@ -676,7 +683,7 @@ static void __init int3_selftest(void)
"int3_selftest_ip:\n\t"
__ASM_SEL(.long, .quad) " 1b\n\t"
".popsection\n\t"
- : : __ASM_SEL_RAW(a, D) (&val) : "memory");
+ : : __ASM_SEL_RAW(a, D) (&val) : INT3_TEST_CLOBBERS);
BUG_ON(val != 1);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/alternatives: Fix int3_emulate_call() selftest stack corruption
2019-07-08 20:55 [PATCH] x86/alternatives: Fix int3_emulate_call() selftest stack corruption Josh Poimboeuf
@ 2019-07-09 12:57 ` Peter Zijlstra
2019-07-09 13:13 ` Josh Poimboeuf
2019-07-09 20:49 ` [tip:x86/urgent] " tip-bot for Peter Zijlstra
0 siblings, 2 replies; 4+ messages in thread
From: Peter Zijlstra @ 2019-07-09 12:57 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, Andy Lutomirski, Ingo Molnar, Thomas Gleixner,
Linus Torvalds, kernel test robot
On Mon, Jul 08, 2019 at 03:55:30PM -0500, Josh Poimboeuf wrote:
> arch/x86/kernel/alternative.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 99ef8b6f9a1a..2644a7b82f96 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -651,6 +651,13 @@ int3_exception_notify(struct notifier_block *self, unsigned long val, void *data
> return NOTIFY_STOP;
> }
>
> +#ifdef CONFIG_X86_32
> +# define INT3_TEST_CLOBBERS "memory", "cc", "ecx", "edx"
> +#else
> +# define INT3_TEST_CLOBBERS "memory", "cc", "rax", "rcx", "rdx", "rsi", "r8", \
> + "r9", "r10", "r11"
> +#endif
> +
> static void __init int3_selftest(void)
> {
> static __initdata struct notifier_block int3_exception_nb = {
> @@ -676,7 +683,7 @@ static void __init int3_selftest(void)
> "int3_selftest_ip:\n\t"
> __ASM_SEL(.long, .quad) " 1b\n\t"
> ".popsection\n\t"
> - : : __ASM_SEL_RAW(a, D) (&val) : "memory");
> + : : __ASM_SEL_RAW(a, D) (&val) : INT3_TEST_CLOBBERS);
>
> BUG_ON(val != 1);
As discussed on IRC; I've changed that to the other variant.
---
Subject: x86/alternatives: Fix int3_emulate_call() selftest stack corruption
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon, 8 Jul 2019 15:55:30 -0500
KASAN shows the following splat during boot:
BUG: KASAN: unknown-crash in unwind_next_frame+0x3f6/0x490
Read of size 8 at addr ffffffff84007db0 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Tainted: G T 5.2.0-rc6-00013-g7457c0d #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
dump_stack+0x19/0x1b
print_address_description+0x1b0/0x2b2
? unwind_next_frame+0x3f6/0x490
__kasan_report+0x10f/0x171
? unwind_next_frame+0x3f6/0x490
kasan_report+0x12/0x1c
__asan_load8+0x54/0x81
unwind_next_frame+0x3f6/0x490
? unwind_dump+0x24e/0x24e
unwind_next_frame+0x1b/0x23
? create_prof_cpu_mask+0x20/0x20
arch_stack_walk+0x68/0xa5
? set_memory_4k+0x2a/0x2c
stack_trace_save+0x7b/0xa0
? stack_trace_consume_entry+0x89/0x89
save_trace+0x3c/0x93
mark_lock+0x1ef/0x9b1
? sched_clock_local+0x86/0xa6
__lock_acquire+0x3ba/0x1bea
? __asan_loadN+0xf/0x11
? mark_held_locks+0x8e/0x8e
? mark_lock+0xb4/0x9b1
? sched_clock_local+0x86/0xa6
lock_acquire+0x122/0x221
? _vm_unmap_aliases+0x141/0x183
__mutex_lock+0xb6/0x731
? _vm_unmap_aliases+0x141/0x183
? sched_clock_cpu+0xac/0xb1
? __mutex_add_waiter+0xae/0xae
? lock_downgrade+0x368/0x368
? _vm_unmap_aliases+0x40/0x183
mutex_lock_nested+0x16/0x18
_vm_unmap_aliases+0x141/0x183
? _vm_unmap_aliases+0x40/0x183
vm_unmap_aliases+0x14/0x16
change_page_attr_set_clr+0x15e/0x2f2
? __set_pages_p+0x111/0x111
? alternative_instructions+0xd8/0x118
? arch_init_ideal_nops+0x181/0x181
set_memory_4k+0x2a/0x2c
check_bugs+0x11fd/0x1298
? l1tf_cmdline+0x1dc/0x1dc
? proc_create_single_data+0x5f/0x6e
? cgroup_init+0x2b1/0x2f6
start_kernel+0x793/0x7eb
? thread_stack_cache_init+0x2e/0x2e
? idt_setup_early_handler+0x70/0xb1
x86_64_start_reservations+0x55/0x76
x86_64_start_kernel+0x87/0xaa
secondary_startup_64+0xa4/0xb0
Memory state around the buggy address:
ffffffff84007c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1
ffffffff84007d00: f1 00 00 00 00 00 00 00 00 00 f2 f2 f2 f3 f3 f3
>ffffffff84007d80: f3 79 be 52 49 79 be 00 00 00 00 00 00 00 00 f1
It turns out that int3_selftest() is corrupting the stack. The problem
is that the KASAN-ified version of int3_magic() is much less trivial
than the C code appears. It clobbers several unexpected registers. So
when the selftest's INT3 is converted to an emulated call to
int3_magic(), the registers are clobbered and Bad Things happen when the
function returns.
Fix this by converting int3_magic() to the trivial ASM function it
should be, avoiding all calling convetion issues. Also add
ASM_CALL_CONSTRAINT to the INT3 ASM, since it contains a 'CALL'.
Fixes: 7457c0da024b ("x86/alternatives: Add int3_emulate_call() selftest")
Cc: x86@kernel.org
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Reported-by: kernel test robot <rong.a.chen@intel.com>
Debugged-by: Josh Poimboeuf <jpoimboe@redhat.com>
[peterz: cribbed changelog from josh]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/kernel/alternative.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -625,10 +625,23 @@ extern struct paravirt_patch_site __star
*
* See entry_{32,64}.S for more details.
*/
-static void __init int3_magic(unsigned int *ptr)
-{
- *ptr = 1;
-}
+
+/*
+ * We define the int3_magic() function in assembly to control the calling
+ * convention such that we can 'call' it from assembly.
+ */
+
+extern void int3_magic(unsigned int *ptr); /* defined in asm */
+
+asm (
+" .pushsection .init.text, \"ax\", @progbits\n"
+" .type int3_magic, @function\n"
+"int3_magic:\n"
+" movl $1, (%" _ASM_ARG1 ")\n"
+" ret\n"
+" .size int3_magic, .-int3_magic\n"
+" .popsection\n"
+);
extern __initdata unsigned long int3_selftest_ip; /* defined in asm below */
@@ -676,7 +689,9 @@ static void __init int3_selftest(void)
"int3_selftest_ip:\n\t"
__ASM_SEL(.long, .quad) " 1b\n\t"
".popsection\n\t"
- : : __ASM_SEL_RAW(a, D) (&val) : "memory");
+ : ASM_CALL_CONSTRAINT
+ : __ASM_SEL_RAW(a, D) (&val)
+ : "memory");
BUG_ON(val != 1);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/alternatives: Fix int3_emulate_call() selftest stack corruption
2019-07-09 12:57 ` Peter Zijlstra
@ 2019-07-09 13:13 ` Josh Poimboeuf
2019-07-09 20:49 ` [tip:x86/urgent] " tip-bot for Peter Zijlstra
1 sibling, 0 replies; 4+ messages in thread
From: Josh Poimboeuf @ 2019-07-09 13:13 UTC (permalink / raw)
To: Peter Zijlstra
Cc: x86, linux-kernel, Andy Lutomirski, Ingo Molnar, Thomas Gleixner,
Linus Torvalds, kernel test robot
On Tue, Jul 09, 2019 at 02:57:44PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 08, 2019 at 03:55:30PM -0500, Josh Poimboeuf wrote:
>
> > arch/x86/kernel/alternative.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> > index 99ef8b6f9a1a..2644a7b82f96 100644
> > --- a/arch/x86/kernel/alternative.c
> > +++ b/arch/x86/kernel/alternative.c
> > @@ -651,6 +651,13 @@ int3_exception_notify(struct notifier_block *self, unsigned long val, void *data
> > return NOTIFY_STOP;
> > }
> >
> > +#ifdef CONFIG_X86_32
> > +# define INT3_TEST_CLOBBERS "memory", "cc", "ecx", "edx"
> > +#else
> > +# define INT3_TEST_CLOBBERS "memory", "cc", "rax", "rcx", "rdx", "rsi", "r8", \
> > + "r9", "r10", "r11"
> > +#endif
> > +
> > static void __init int3_selftest(void)
> > {
> > static __initdata struct notifier_block int3_exception_nb = {
> > @@ -676,7 +683,7 @@ static void __init int3_selftest(void)
> > "int3_selftest_ip:\n\t"
> > __ASM_SEL(.long, .quad) " 1b\n\t"
> > ".popsection\n\t"
> > - : : __ASM_SEL_RAW(a, D) (&val) : "memory");
> > + : : __ASM_SEL_RAW(a, D) (&val) : INT3_TEST_CLOBBERS);
> >
> > BUG_ON(val != 1);
>
> As discussed on IRC; I've changed that to the other variant.
>
> ---
> Subject: x86/alternatives: Fix int3_emulate_call() selftest stack corruption
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Mon, 8 Jul 2019 15:55:30 -0500
>
> KASAN shows the following splat during boot:
>
> BUG: KASAN: unknown-crash in unwind_next_frame+0x3f6/0x490
> Read of size 8 at addr ffffffff84007db0 by task swapper/0
>
> CPU: 0 PID: 0 Comm: swapper Tainted: G T 5.2.0-rc6-00013-g7457c0d #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> Call Trace:
> dump_stack+0x19/0x1b
> print_address_description+0x1b0/0x2b2
> ? unwind_next_frame+0x3f6/0x490
> __kasan_report+0x10f/0x171
> ? unwind_next_frame+0x3f6/0x490
> kasan_report+0x12/0x1c
> __asan_load8+0x54/0x81
> unwind_next_frame+0x3f6/0x490
> ? unwind_dump+0x24e/0x24e
> unwind_next_frame+0x1b/0x23
> ? create_prof_cpu_mask+0x20/0x20
> arch_stack_walk+0x68/0xa5
> ? set_memory_4k+0x2a/0x2c
> stack_trace_save+0x7b/0xa0
> ? stack_trace_consume_entry+0x89/0x89
> save_trace+0x3c/0x93
> mark_lock+0x1ef/0x9b1
> ? sched_clock_local+0x86/0xa6
> __lock_acquire+0x3ba/0x1bea
> ? __asan_loadN+0xf/0x11
> ? mark_held_locks+0x8e/0x8e
> ? mark_lock+0xb4/0x9b1
> ? sched_clock_local+0x86/0xa6
> lock_acquire+0x122/0x221
> ? _vm_unmap_aliases+0x141/0x183
> __mutex_lock+0xb6/0x731
> ? _vm_unmap_aliases+0x141/0x183
> ? sched_clock_cpu+0xac/0xb1
> ? __mutex_add_waiter+0xae/0xae
> ? lock_downgrade+0x368/0x368
> ? _vm_unmap_aliases+0x40/0x183
> mutex_lock_nested+0x16/0x18
> _vm_unmap_aliases+0x141/0x183
> ? _vm_unmap_aliases+0x40/0x183
> vm_unmap_aliases+0x14/0x16
> change_page_attr_set_clr+0x15e/0x2f2
> ? __set_pages_p+0x111/0x111
> ? alternative_instructions+0xd8/0x118
> ? arch_init_ideal_nops+0x181/0x181
> set_memory_4k+0x2a/0x2c
> check_bugs+0x11fd/0x1298
> ? l1tf_cmdline+0x1dc/0x1dc
> ? proc_create_single_data+0x5f/0x6e
> ? cgroup_init+0x2b1/0x2f6
> start_kernel+0x793/0x7eb
> ? thread_stack_cache_init+0x2e/0x2e
> ? idt_setup_early_handler+0x70/0xb1
> x86_64_start_reservations+0x55/0x76
> x86_64_start_kernel+0x87/0xaa
> secondary_startup_64+0xa4/0xb0
>
> Memory state around the buggy address:
> ffffffff84007c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1
> ffffffff84007d00: f1 00 00 00 00 00 00 00 00 00 f2 f2 f2 f3 f3 f3
> >ffffffff84007d80: f3 79 be 52 49 79 be 00 00 00 00 00 00 00 00 f1
>
> It turns out that int3_selftest() is corrupting the stack. The problem
> is that the KASAN-ified version of int3_magic() is much less trivial
> than the C code appears. It clobbers several unexpected registers. So
> when the selftest's INT3 is converted to an emulated call to
> int3_magic(), the registers are clobbered and Bad Things happen when the
> function returns.
>
> Fix this by converting int3_magic() to the trivial ASM function it
> should be, avoiding all calling convetion issues. Also add
> ASM_CALL_CONSTRAINT to the INT3 ASM, since it contains a 'CALL'.
>
> Fixes: 7457c0da024b ("x86/alternatives: Add int3_emulate_call() selftest")
> Cc: x86@kernel.org
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andy Lutomirski <luto@kernel.org>
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> Debugged-by: Josh Poimboeuf <jpoimboe@redhat.com>
> [peterz: cribbed changelog from josh]
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> arch/x86/kernel/alternative.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -625,10 +625,23 @@ extern struct paravirt_patch_site __star
> *
> * See entry_{32,64}.S for more details.
> */
> -static void __init int3_magic(unsigned int *ptr)
> -{
> - *ptr = 1;
> -}
> +
> +/*
> + * We define the int3_magic() function in assembly to control the calling
> + * convention such that we can 'call' it from assembly.
> + */
> +
> +extern void int3_magic(unsigned int *ptr); /* defined in asm */
> +
> +asm (
> +" .pushsection .init.text, \"ax\", @progbits\n"
> +" .type int3_magic, @function\n"
> +"int3_magic:\n"
> +" movl $1, (%" _ASM_ARG1 ")\n"
> +" ret\n"
> +" .size int3_magic, .-int3_magic\n"
> +" .popsection\n"
> +);
>
> extern __initdata unsigned long int3_selftest_ip; /* defined in asm below */
>
> @@ -676,7 +689,9 @@ static void __init int3_selftest(void)
> "int3_selftest_ip:\n\t"
> __ASM_SEL(.long, .quad) " 1b\n\t"
> ".popsection\n\t"
> - : : __ASM_SEL_RAW(a, D) (&val) : "memory");
> + : ASM_CALL_CONSTRAINT
> + : __ASM_SEL_RAW(a, D) (&val)
> + : "memory");
>
> BUG_ON(val != 1);
>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:x86/urgent] x86/alternatives: Fix int3_emulate_call() selftest stack corruption
2019-07-09 12:57 ` Peter Zijlstra
2019-07-09 13:13 ` Josh Poimboeuf
@ 2019-07-09 20:49 ` tip-bot for Peter Zijlstra
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Peter Zijlstra @ 2019-07-09 20:49 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, torvalds, peterz, tglx, mingo, hpa, rong.a.chen,
luto, jpoimboe
Commit-ID: ecc606103837b98a2b665e8f14e533a6c72bbdc0
Gitweb: https://git.kernel.org/tip/ecc606103837b98a2b665e8f14e533a6c72bbdc0
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 8 Jul 2019 15:55:30 -0500
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 9 Jul 2019 22:39:15 +0200
x86/alternatives: Fix int3_emulate_call() selftest stack corruption
KASAN shows the following splat during boot:
BUG: KASAN: unknown-crash in unwind_next_frame+0x3f6/0x490
Read of size 8 at addr ffffffff84007db0 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Tainted: G T 5.2.0-rc6-00013-g7457c0d #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
dump_stack+0x19/0x1b
print_address_description+0x1b0/0x2b2
__kasan_report+0x10f/0x171
kasan_report+0x12/0x1c
__asan_load8+0x54/0x81
unwind_next_frame+0x3f6/0x490
unwind_next_frame+0x1b/0x23
arch_stack_walk+0x68/0xa5
stack_trace_save+0x7b/0xa0
save_trace+0x3c/0x93
mark_lock+0x1ef/0x9b1
lock_acquire+0x122/0x221
__mutex_lock+0xb6/0x731
mutex_lock_nested+0x16/0x18
_vm_unmap_aliases+0x141/0x183
vm_unmap_aliases+0x14/0x16
change_page_attr_set_clr+0x15e/0x2f2
set_memory_4k+0x2a/0x2c
check_bugs+0x11fd/0x1298
start_kernel+0x793/0x7eb
x86_64_start_reservations+0x55/0x76
x86_64_start_kernel+0x87/0xaa
secondary_startup_64+0xa4/0xb0
Memory state around the buggy address:
ffffffff84007c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1
ffffffff84007d00: f1 00 00 00 00 00 00 00 00 00 f2 f2 f2 f3 f3 f3
>ffffffff84007d80: f3 79 be 52 49 79 be 00 00 00 00 00 00 00 00 f1
It turns out that int3_selftest() is corrupting the stack. The problem is
that the KASAN-ified version of int3_magic() is much less trivial than the
C code appears. It clobbers several unexpected registers. So when the
selftest's INT3 is converted to an emulated call to int3_magic(), the
registers are clobbered and Bad Things happen when the function returns.
Fix this by converting int3_magic() to the trivial ASM function it should
be, avoiding all calling convention issues. Also add ASM_CALL_CONSTRAINT to
the INT3 ASM, since it contains a 'CALL'.
[peterz: cribbed changelog from josh]
Fixes: 7457c0da024b ("x86/alternatives: Add int3_emulate_call() selftest")
Reported-by: kernel test robot <rong.a.chen@intel.com>
Debugged-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Link: https://lkml.kernel.org/r/20190709125744.GB3402@hirez.programming.kicks-ass.net
---
arch/x86/kernel/alternative.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 99ef8b6f9a1a..ccd32013c47a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -625,10 +625,23 @@ extern struct paravirt_patch_site __start_parainstructions[],
*
* See entry_{32,64}.S for more details.
*/
-static void __init int3_magic(unsigned int *ptr)
-{
- *ptr = 1;
-}
+
+/*
+ * We define the int3_magic() function in assembly to control the calling
+ * convention such that we can 'call' it from assembly.
+ */
+
+extern void int3_magic(unsigned int *ptr); /* defined in asm */
+
+asm (
+" .pushsection .init.text, \"ax\", @progbits\n"
+" .type int3_magic, @function\n"
+"int3_magic:\n"
+" movl $1, (%" _ASM_ARG1 ")\n"
+" ret\n"
+" .size int3_magic, .-int3_magic\n"
+" .popsection\n"
+);
extern __initdata unsigned long int3_selftest_ip; /* defined in asm below */
@@ -676,7 +689,9 @@ static void __init int3_selftest(void)
"int3_selftest_ip:\n\t"
__ASM_SEL(.long, .quad) " 1b\n\t"
".popsection\n\t"
- : : __ASM_SEL_RAW(a, D) (&val) : "memory");
+ : ASM_CALL_CONSTRAINT
+ : __ASM_SEL_RAW(a, D) (&val)
+ : "memory");
BUG_ON(val != 1);
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-09 20:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-08 20:55 [PATCH] x86/alternatives: Fix int3_emulate_call() selftest stack corruption Josh Poimboeuf
2019-07-09 12:57 ` Peter Zijlstra
2019-07-09 13:13 ` Josh Poimboeuf
2019-07-09 20:49 ` [tip:x86/urgent] " tip-bot for Peter Zijlstra
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.