* [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 [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 [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome