* [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.