All of lore.kernel.org
 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 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.