linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).