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 <[email protected]> > Cc: Mark Rutland <[email protected]> > Cc: Catalin Marinas <[email protected]> > Cc: Andrey Ryabinin <[email protected]> > Cc: Lorenzo Pieralisi <[email protected]> > Cc: Alexander Potapenko <[email protected]> > Cc: Will Deacon <[email protected]> > Cc: Ingo Molnar <[email protected]> > Cc: Andrew Morton <[email protected]> > Cc: Thomas Gleixner <[email protected]> > Cc: "H. Peter Anvin" <[email protected]> > Cc: Ananth N Mavinakayanahalli <[email protected]> > Cc: Anil S Keshavamurthy <[email protected]> > Cc: "David S. Miller" <[email protected]> > Cc: Masami Hiramatsu <[email protected]> > Cc: [email protected] > Cc: [email protected]
Both the core and arm64 parts look right to me, so FWIW: Reviewed-by: Mark Rutland <[email protected]> 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 >
