All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] kprobes: unpoison stack in jprobe_return() for KASAN
@ 2016-10-14 10:53 Dmitry Vyukov
  2016-10-14 11:15 ` Mark Rutland
  2016-10-14 11:25 ` Will Deacon
  0 siblings, 2 replies; 4+ messages in thread
From: Dmitry Vyukov @ 2016-10-14 10:53 UTC (permalink / raw)
  To: rostedt, mingo, akpm
  Cc: linux-kernel, ryabinin.a.a, surovegin, Dmitry Vyukov,
	Mark Rutland, Catalin Marinas, Lorenzo Pieralisi,
	Alexander Potapenko, Will Deacon, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin, Ananth N Mavinakayanahalli, Anil S Keshavamurthy,
	David S. Miller, Masami Hiramatsu, x86, kasan-dev

KASAN stack instrumentation poisons stack redzones on function entry
and unpoisons them on function exit. If a function exits abnormally
(e.g. with a longjmp like jprobe_return()), stack redzones are left
poisoned. Later this leads to random KASAN false reports.

Unpoison stack redzones in the frames we are going to jump over
before doing actual longjmp in jprobe_return().

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: x86@kernel.org
Cc: kasan-dev@googlegroups.com

--

Changes since v1:
 - leave kasan_unpoison_remaining_stack() intact
 - instead add kasan_unpoison_stack_above_sp_to()
 - rename kasan_unpoison_remaining_stack() to kasan_unpoison_task_stack_below()

Changes since v2:
 - fix build by adding return type to kasan_unpoison_stack_above_sp_to
   (tested v2 with it, but forgot to git add)

I observe false positives due to this in sctp code.
sctp uses jprobe_return() in jsctp_sf_eat_sack().
The stray 0xf4 in shadow memory are stack redzones.

[  376.492209] ==================================================================
[  376.500368] BUG: KASAN: stack-out-of-bounds in memcmp+0xe9/0x150 at addr ffff88005e48f480
[  376.509522] Read of size 1 by task syz-executor/18535
[  376.515249] page:ffffea00017923c0 count:0 mapcount:0 mapping:          (null) index:0x0
[  376.524377] flags: 0x1fffc0000000000()
[  376.528645] page dumped because: kasan: bad access detected
[  376.534939] CPU: 1 PID: 18535 Comm: syz-executor Not tainted 4.8.0+ #28
[  376.542375] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
[  376.552669]  ffff88005e48f2d0 ffffffff82d2b849 ffffffff0bc91e90 fffffbfff10971e8
[  376.561599]  ffffed000bc91e90 ffffed000bc91e90 0000000000000001 0000000000000000
[  376.570486]  ffff88005e48f480 ffff88005e48f350 ffffffff817d3169 ffff88005e48f370
[  376.579348] Call Trace:
[  376.582196]  [<ffffffff82d2b849>] dump_stack+0x12e/0x185
[  376.588190]  [<ffffffff817d3169>] kasan_report+0x489/0x4b0
[  376.594378]  [<ffffffff82d49529>] ? memcmp+0xe9/0x150
[  376.600099]  [<ffffffff8126377e>] ? update_stack_state.constprop.4+0xde/0x150
[  376.608187]  [<ffffffff817d31a9>] __asan_report_load1_noabort+0x19/0x20
[  376.615620]  [<ffffffff82d49529>] memcmp+0xe9/0x150
[  376.621148]  [<ffffffff82df7486>] depot_save_stack+0x176/0x5c0
[  376.627722]  [<ffffffff85b03f19>] ? skb_free_head+0x79/0xb0
[  376.634006]  [<ffffffff817d2031>] save_stack+0xb1/0xd0
[  376.639805]  [<ffffffff811fc8ab>] ? save_stack_trace+0x1b/0x20
[  376.646369]  [<ffffffff817d1fc6>] ? save_stack+0x46/0xd0
[  376.652366]  [<ffffffff817d27f2>] ? kasan_slab_free+0x72/0xc0
[  376.658837]  [<ffffffff817d05b8>] ? kfree+0xc8/0x2a0
[  376.664454]  [<ffffffff85b03f19>] ? skb_free_head+0x79/0xb0
[  376.670736]  [<ffffffff85b0900a>] ? skb_release_data+0x37a/0x420
[  376.677517]  [<ffffffff85b090ff>] ? skb_release_all+0x4f/0x60
[  376.683992]  [<ffffffff85b11348>] ? consume_skb+0x138/0x370
[  376.690284]  [<ffffffff8676ad7b>] ? sctp_chunk_put+0xcb/0x180
[  376.696761]  [<ffffffff8676ae88>] ? sctp_chunk_free+0x58/0x70
[  376.703234]  [<ffffffff8677fa5f>] ? sctp_inq_pop+0x68f/0xef0
[  376.709616]  [<ffffffff8675ee36>] ? sctp_assoc_bh_rcv+0xd6/0x4b0
[  376.716379]  [<ffffffff8677f2c1>] ? sctp_inq_push+0x131/0x190
[  376.722946]  [<ffffffff867bad69>] ? sctp_backlog_rcv+0xe9/0xa20
[  376.729615]  [<ffffffff85af70dc>] ? __release_sock+0x12c/0x3a0
[  376.736186]  [<ffffffff85af73ae>] ? release_sock+0x5e/0x1c0
[  376.742465]  [<ffffffff8679fc62>] ? sctp_sendmsg+0xd82/0x2e00
[  376.748935]  [<ffffffff85f83c43>] ? inet_sendmsg+0x303/0x4c0
[  376.755318]  [<ffffffff85aeab0f>] ? sock_sendmsg+0xcf/0x110
[  376.761599]  [<ffffffff85aebc1d>] ? SYSC_sendto+0x20d/0x340
[  376.767882]  [<ffffffff85aee175>] ? SyS_sendto+0x45/0x60
[  376.773885]  [<ffffffff81008783>] ? do_syscall_64+0x1d3/0x620
[  376.780372]  [<ffffffff86d8e9cd>] ? entry_SYSCALL64_slow_path+0x25/0x25
[  376.787832]  [<ffffffff86751c79>] ? sctp_do_sm+0x3689/0x4e90
[  376.794215]  [<ffffffff81467040>] ? debug_check_no_locks_freed+0x3c0/0x3c0
[  376.801948]  [<ffffffff8674e5f0>] ? sctp_do_8_2_transport_strike.isra.19+0x900/0x900
[  376.810918]  [<ffffffff81466ed0>] ? debug_check_no_locks_freed+0x250/0x3c0
[  376.818650]  [<ffffffff817d27f2>] kasan_slab_free+0x72/0xc0
[  376.824945]  [<ffffffff817d05b8>] kfree+0xc8/0x2a0
[  376.830374]  [<ffffffff85b03f19>] skb_free_head+0x79/0xb0
[  376.836466]  [<ffffffff85b0900a>] skb_release_data+0x37a/0x420
[  376.843036]  [<ffffffff85b090ff>] skb_release_all+0x4f/0x60
[  376.849319]  [<ffffffff85b11348>] consume_skb+0x138/0x370
[  376.855411]  [<ffffffff8676ad7b>] sctp_chunk_put+0xcb/0x180
[  376.861702]  [<ffffffff8676ae88>] sctp_chunk_free+0x58/0x70
[  376.867990]  [<ffffffff8677fa5f>] sctp_inq_pop+0x68f/0xef0
[  376.874182]  [<ffffffff8675ee36>] sctp_assoc_bh_rcv+0xd6/0x4b0
[  376.880759]  [<ffffffff8677f2c1>] sctp_inq_push+0x131/0x190
[  376.887055]  [<ffffffff867bad69>] sctp_backlog_rcv+0xe9/0xa20
[  376.893532]  [<ffffffff814667dd>] ? trace_hardirqs_on+0xd/0x10
[  376.900107]  [<ffffffff81369bed>] ? __local_bh_enable_ip+0xad/0x190
[  376.907161]  [<ffffffff85af70dc>] __release_sock+0x12c/0x3a0
[  376.913540]  [<ffffffff85af73ae>] release_sock+0x5e/0x1c0
[  376.919922]  [<ffffffff8679fc62>] sctp_sendmsg+0xd82/0x2e00
[  376.926209]  [<ffffffff8679eee0>] ? sctp_id2assoc+0x330/0x330
[  376.932696]  [<ffffffff81467040>] ? debug_check_no_locks_freed+0x3c0/0x3c0
[  376.940417]  [<ffffffff814a6127>] ? debug_lockdep_rcu_enabled+0x77/0x90
[  376.947858]  [<ffffffff814a6127>] ? debug_lockdep_rcu_enabled+0x77/0x90
[  376.955293]  [<ffffffff85f83940>] ? inet_recvmsg+0x4b0/0x4b0
[  376.961675]  [<ffffffff85f83b48>] ? inet_sendmsg+0x208/0x4c0
[  376.968064]  [<ffffffff85f83c43>] inet_sendmsg+0x303/0x4c0
[  376.974261]  [<ffffffff85f839b8>] ? inet_sendmsg+0x78/0x4c0
[  376.980546]  [<ffffffff85f83940>] ? inet_recvmsg+0x4b0/0x4b0
[  376.986929]  [<ffffffff85aeab0f>] sock_sendmsg+0xcf/0x110
[  376.993025]  [<ffffffff85aebc1d>] SYSC_sendto+0x20d/0x340
[  376.999125]  [<ffffffff85aeba10>] ? SYSC_connect+0x2e0/0x2e0
[  377.005510]  [<ffffffff8188cc33>] ? __fd_install+0x233/0x560
[  377.011894]  [<ffffffff81007b93>] ? perf_trace_sys_enter+0x443/0x9d0
[  377.019051]  [<ffffffff81007750>] ? syscall_trace_enter+0xe00/0xe00
[  377.026109]  [<ffffffff8158887f>] ? __sanitizer_cov_trace_pc+0x4f/0x60
[  377.033466]  [<ffffffff85aee175>] SyS_sendto+0x45/0x60
[  377.039278]  [<ffffffff85aee130>] ? SyS_getpeername+0x30/0x30
[  377.045752]  [<ffffffff81008783>] do_syscall_64+0x1d3/0x620
[  377.052035]  [<ffffffff8100501a>] ? trace_hardirqs_on_thunk+0x1a/0x1c
[  377.059281]  [<ffffffff86d8e9cd>] entry_SYSCALL64_slow_path+0x25/0x25
[  377.066509] Memory state around the buggy address:
[  377.071935]  ffff88005e48f380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  377.080024]  ffff88005e48f400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  377.088117] >ffff88005e48f480: f4 f4 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  377.096205]                    ^
[  377.099900]  ffff88005e48f500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  377.107994]  ffff88005e48f580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  377.116079] ==================================================================
---
 arch/arm64/kernel/sleep.S      |  2 +-
 arch/x86/kernel/kprobes/core.c |  4 ++++
 include/linux/kasan.h          |  2 ++
 mm/kasan/kasan.c               | 19 +++++++++++++++++--
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
index b8799e7..1bec41b 100644
--- a/arch/arm64/kernel/sleep.S
+++ b/arch/arm64/kernel/sleep.S
@@ -135,7 +135,7 @@ ENTRY(_cpu_resume)
 
 #ifdef CONFIG_KASAN
 	mov	x0, sp
-	bl	kasan_unpoison_remaining_stack
+	bl	kasan_unpoison_task_stack_below
 #endif
 
 	ldp	x19, x20, [x29, #16]
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 28cee01..22a462a 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -50,6 +50,7 @@
 #include <linux/kallsyms.h>
 #include <linux/ftrace.h>
 #include <linux/frame.h>
+#include <linux/kasan.h>
 
 #include <asm/text-patching.h>
 #include <asm/cacheflush.h>
@@ -1080,6 +1081,9 @@ void jprobe_return(void)
 {
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
+	/* Unpoison stack redzones in the frames we are going to jump over. */
+	kasan_unpoison_stack_above_sp_to(kcb->jprobe_saved_sp);
+
 	asm volatile (
 #ifdef CONFIG_X86_64
 			"       xchg   %%rbx,%%rsp	\n"
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index d600303..820c0ad 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -44,6 +44,7 @@ static inline void kasan_disable_current(void)
 void kasan_unpoison_shadow(const void *address, size_t size);
 
 void kasan_unpoison_task_stack(struct task_struct *task);
+void kasan_unpoison_stack_above_sp_to(const void *watermark);
 
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
@@ -85,6 +86,7 @@ size_t kasan_metadata_size(struct kmem_cache *cache);
 static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
 
 static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
+static inline void kasan_unpoison_stack_above_sp_to(const void *watermark) {}
 
 static inline void kasan_enable_current(void) {}
 static inline void kasan_disable_current(void) {}
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 88af13c..52d7ff1 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -77,9 +77,24 @@ void kasan_unpoison_task_stack(struct task_struct *task)
 }
 
 /* Unpoison the stack for the current task beyond a watermark sp value. */
-asmlinkage void kasan_unpoison_remaining_stack(void *sp)
+asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)
 {
-	__kasan_unpoison_stack(current, sp);
+	__kasan_unpoison_stack(current, watermark);
+}
+
+/*
+ * Clear all poison for the region between the current SP and a provided
+ * watermark value, as is sometimes required prior to hand-crafted asm function
+ * returns in the middle of functions.
+ */
+void kasan_unpoison_stack_above_sp_to(const void *watermark)
+{
+	const void *sp = (void *)current_stack_pointer();
+	size_t size = watermark - sp;
+
+	if (WARN_ON(sp > watermark))
+		return;
+	kasan_unpoison_shadow(sp, size);
 }
 
 /*
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] kprobes: unpoison stack in jprobe_return() for KASAN
  2016-10-14 10:53 [PATCH v3] kprobes: unpoison stack in jprobe_return() for KASAN Dmitry Vyukov
@ 2016-10-14 11:15 ` Mark Rutland
  2016-10-14 11:25 ` Will Deacon
  1 sibling, 0 replies; 4+ messages in thread
From: Mark Rutland @ 2016-10-14 11:15 UTC (permalink / raw)
  To: Dmitry Vyukov, Will Deacon, Catalin Marinas
  Cc: rostedt, mingo, akpm, linux-kernel, ryabinin.a.a, surovegin,
	Lorenzo Pieralisi, Alexander Potapenko, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin, Ananth N Mavinakayanahalli,
	Anil S Keshavamurthy, David S. Miller, Masami Hiramatsu, x86,
	kasan-dev

On Fri, Oct 14, 2016 at 12:53:56PM +0200, Dmitry Vyukov wrote:
> KASAN stack instrumentation poisons stack redzones on function entry
> and unpoisons them on function exit. If a function exits abnormally
> (e.g. with a longjmp like jprobe_return()), stack redzones are left
> poisoned. Later this leads to random KASAN false reports.
> 
> Unpoison stack redzones in the frames we are going to jump over
> before doing actual longjmp in jprobe_return().
> 
> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: x86@kernel.org
> Cc: kasan-dev@googlegroups.com

Both the core and arm64 parts look right to me, so FWIW:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Catalin, Will, are you happy to ack the arm64 part? A core function got
renamed, and we have to update the call site in sleep.S, but there
should be no functional change. The rest of the patch adds some generic
infrastructure required by x86.

Thanks,
Mark.

> ---
>  arch/arm64/kernel/sleep.S      |  2 +-
>  arch/x86/kernel/kprobes/core.c |  4 ++++
>  include/linux/kasan.h          |  2 ++
>  mm/kasan/kasan.c               | 19 +++++++++++++++++--
>  4 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
> index b8799e7..1bec41b 100644
> --- a/arch/arm64/kernel/sleep.S
> +++ b/arch/arm64/kernel/sleep.S
> @@ -135,7 +135,7 @@ ENTRY(_cpu_resume)
>  
>  #ifdef CONFIG_KASAN
>  	mov	x0, sp
> -	bl	kasan_unpoison_remaining_stack
> +	bl	kasan_unpoison_task_stack_below
>  #endif
>  
>  	ldp	x19, x20, [x29, #16]
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 28cee01..22a462a 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -50,6 +50,7 @@
>  #include <linux/kallsyms.h>
>  #include <linux/ftrace.h>
>  #include <linux/frame.h>
> +#include <linux/kasan.h>
>  
>  #include <asm/text-patching.h>
>  #include <asm/cacheflush.h>
> @@ -1080,6 +1081,9 @@ void jprobe_return(void)
>  {
>  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>  
> +	/* Unpoison stack redzones in the frames we are going to jump over. */
> +	kasan_unpoison_stack_above_sp_to(kcb->jprobe_saved_sp);
> +
>  	asm volatile (
>  #ifdef CONFIG_X86_64
>  			"       xchg   %%rbx,%%rsp	\n"
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index d600303..820c0ad 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -44,6 +44,7 @@ static inline void kasan_disable_current(void)
>  void kasan_unpoison_shadow(const void *address, size_t size);
>  
>  void kasan_unpoison_task_stack(struct task_struct *task);
> +void kasan_unpoison_stack_above_sp_to(const void *watermark);
>  
>  void kasan_alloc_pages(struct page *page, unsigned int order);
>  void kasan_free_pages(struct page *page, unsigned int order);
> @@ -85,6 +86,7 @@ size_t kasan_metadata_size(struct kmem_cache *cache);
>  static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
>  
>  static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
> +static inline void kasan_unpoison_stack_above_sp_to(const void *watermark) {}
>  
>  static inline void kasan_enable_current(void) {}
>  static inline void kasan_disable_current(void) {}
> diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
> index 88af13c..52d7ff1 100644
> --- a/mm/kasan/kasan.c
> +++ b/mm/kasan/kasan.c
> @@ -77,9 +77,24 @@ void kasan_unpoison_task_stack(struct task_struct *task)
>  }
>  
>  /* Unpoison the stack for the current task beyond a watermark sp value. */
> -asmlinkage void kasan_unpoison_remaining_stack(void *sp)
> +asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)
>  {
> -	__kasan_unpoison_stack(current, sp);
> +	__kasan_unpoison_stack(current, watermark);
> +}
> +
> +/*
> + * Clear all poison for the region between the current SP and a provided
> + * watermark value, as is sometimes required prior to hand-crafted asm function
> + * returns in the middle of functions.
> + */
> +void kasan_unpoison_stack_above_sp_to(const void *watermark)
> +{
> +	const void *sp = (void *)current_stack_pointer();
> +	size_t size = watermark - sp;
> +
> +	if (WARN_ON(sp > watermark))
> +		return;
> +	kasan_unpoison_shadow(sp, size);
>  }
>  
>  /*
> -- 
> 2.8.0.rc3.226.g39d4020
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] kprobes: unpoison stack in jprobe_return() for KASAN
  2016-10-14 10:53 [PATCH v3] kprobes: unpoison stack in jprobe_return() for KASAN Dmitry Vyukov
  2016-10-14 11:15 ` Mark Rutland
@ 2016-10-14 11:25 ` Will Deacon
  2016-10-14 11:55   ` Dmitry Vyukov
  1 sibling, 1 reply; 4+ messages in thread
From: Will Deacon @ 2016-10-14 11:25 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: rostedt, mingo, akpm, linux-kernel, ryabinin.a.a, surovegin,
	Mark Rutland, Catalin Marinas, Lorenzo Pieralisi,
	Alexander Potapenko, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin, Ananth N Mavinakayanahalli, Anil S Keshavamurthy,
	David S. Miller, Masami Hiramatsu, x86, kasan-dev

On Fri, Oct 14, 2016 at 12:53:56PM +0200, Dmitry Vyukov wrote:
> KASAN stack instrumentation poisons stack redzones on function entry
> and unpoisons them on function exit. If a function exits abnormally
> (e.g. with a longjmp like jprobe_return()), stack redzones are left
> poisoned. Later this leads to random KASAN false reports.
> 
> Unpoison stack redzones in the frames we are going to jump over
> before doing actual longjmp in jprobe_return().
> 
> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: x86@kernel.org
> Cc: kasan-dev@googlegroups.com
> 
> --
> 
> Changes since v1:
>  - leave kasan_unpoison_remaining_stack() intact
>  - instead add kasan_unpoison_stack_above_sp_to()
>  - rename kasan_unpoison_remaining_stack() to kasan_unpoison_task_stack_below()
> 
> Changes since v2:
>  - fix build by adding return type to kasan_unpoison_stack_above_sp_to
>    (tested v2 with it, but forgot to git add)

I get build warnings with this patch applied and KASAN enabled:

mm/kasan/kasan.c: In function ‘kasan_unpoison_task_stack_below’:
mm/kasan/kasan.c:82:34: warning: passing argument 2 of ‘__kasan_unpoison_stack’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  __kasan_unpoison_stack(current, watermark);
                                  ^~~~~~~~~
mm/kasan/kasan.c:65:13: note: expected ‘void *’ but argument is of type ‘const void *’
 static void __kasan_unpoison_stack(struct task_struct *task, void *sp)
             ^~~~~~~~~~~~~~~~~~~~~~
mm/kasan/kasan.c: In function ‘kasan_unpoison_stack_above_sp_to’:
mm/kasan/kasan.c:92:27: error: called object ‘current_stack_pointer’ is not a function or function pointer
  const void *sp = (void *)current_stack_pointer();
                           ^~~~~~~~~~~~~~~~~~~~~
In file included from ./include/linux/thread_info.h:54:0,
                 from ./include/asm-generic/preempt.h:4,
                 from ./arch/arm64/include/generated/asm/preempt.h:1,
                 from ./include/linux/preempt.h:59,
                 from ./include/linux/interrupt.h:8,
                 from mm/kasan/kasan.c:20:
./arch/arm64/include/asm/thread_info.h:69:24: note: declared here
 register unsigned long current_stack_pointer asm ("sp");

Will

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] kprobes: unpoison stack in jprobe_return() for KASAN
  2016-10-14 11:25 ` Will Deacon
@ 2016-10-14 11:55   ` Dmitry Vyukov
  0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Vyukov @ 2016-10-14 11:55 UTC (permalink / raw)
  To: Will Deacon
  Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, LKML,
	Andrey Ryabinin, Eugene Surovegin, Mark Rutland, Catalin Marinas,
	Lorenzo Pieralisi, Alexander Potapenko, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin, Ananth N Mavinakayanahalli,
	Anil S Keshavamurthy, David S. Miller, Masami Hiramatsu, x86,
	kasan-dev

On Fri, Oct 14, 2016 at 1:25 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Oct 14, 2016 at 12:53:56PM +0200, Dmitry Vyukov wrote:
>> KASAN stack instrumentation poisons stack redzones on function entry
>> and unpoisons them on function exit. If a function exits abnormally
>> (e.g. with a longjmp like jprobe_return()), stack redzones are left
>> poisoned. Later this leads to random KASAN false reports.
>>
>> Unpoison stack redzones in the frames we are going to jump over
>> before doing actual longjmp in jprobe_return().
>>
>> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Alexander Potapenko <glider@google.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
>> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Cc: x86@kernel.org
>> Cc: kasan-dev@googlegroups.com
>>
>> --
>>
>> Changes since v1:
>>  - leave kasan_unpoison_remaining_stack() intact
>>  - instead add kasan_unpoison_stack_above_sp_to()
>>  - rename kasan_unpoison_remaining_stack() to kasan_unpoison_task_stack_below()
>>
>> Changes since v2:
>>  - fix build by adding return type to kasan_unpoison_stack_above_sp_to
>>    (tested v2 with it, but forgot to git add)
>
> I get build warnings with this patch applied and KASAN enabled:
>
> mm/kasan/kasan.c: In function ‘kasan_unpoison_task_stack_below’:
> mm/kasan/kasan.c:82:34: warning: passing argument 2 of ‘__kasan_unpoison_stack’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>   __kasan_unpoison_stack(current, watermark);
>                                   ^~~~~~~~~
> mm/kasan/kasan.c:65:13: note: expected ‘void *’ but argument is of type ‘const void *’
>  static void __kasan_unpoison_stack(struct task_struct *task, void *sp)
>              ^~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/kasan.c: In function ‘kasan_unpoison_stack_above_sp_to’:
> mm/kasan/kasan.c:92:27: error: called object ‘current_stack_pointer’ is not a function or function pointer
>   const void *sp = (void *)current_stack_pointer();
>                            ^~~~~~~~~~~~~~~~~~~~~


Mailed v4.

Thanks

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-10-14 11:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-14 10:53 [PATCH v3] kprobes: unpoison stack in jprobe_return() for KASAN Dmitry Vyukov
2016-10-14 11:15 ` Mark Rutland
2016-10-14 11:25 ` Will Deacon
2016-10-14 11:55   ` Dmitry Vyukov

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.