lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20151005112341.GA1101@gmail.com> Date: Mon, 5 Oct 2015 13:23:41 +0200 From: Ingo Molnar <mingo@...nel.org> To: Andrey Ryabinin <aryabinin@...tuozzo.com> Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org, linux-kernel@...r.kernel.org, Andy Lutomirski <luto@...capital.net>, Andrey Konovalov <andreyknvl@...gle.com>, Kostya Serebryany <kcc@...gle.com>, Alexander Potapenko <glider@...gle.com>, kasan-dev <kasan-dev@...glegroups.com>, Borislav Petkov <bp@...en8.de>, Denys Vlasenko <dvlasenk@...hat.com>, Andi Kleen <ak@...ux.intel.com>, Dmitry Vyukov <dvyukov@...gle.com>, Sasha Levin <sasha.levin@...cle.com>, Wolfram Gloger <wmglo@...t.med.uni-muenchen.de>, Linus Torvalds <torvalds@...ux-foundation.org>, Andrew Morton <akpm@...ux-foundation.org> Subject: Re: [PATCH] x86/process: Silence KASAN warnings in get_wchan() * Andrey Ryabinin <aryabinin@...tuozzo.com> wrote: > get_wchan() is racy by design, it may access volatile stack > of running task, thus it may access redzone in a stack frame > and cause KASAN to warn about this. > > Use kasan_disable_current()/kasan_enable_current() to silence > these warnings. > > Reported-by: Sasha Levin <sasha.levin@...cle.com> > Signed-off-by: Andrey Ryabinin <aryabinin@...tuozzo.com> > --- > > Perhaps it would be better to add something like this: > READ_ONCE_NOCHECK() > { > kasan_disable_current(); > READ_ONCE(); > kasan_enable_current(); > } > ? > > arch/x86/kernel/process.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c > index 39e585a..0488eb9 100644 > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -13,6 +13,7 @@ > #include <linux/random.h> > #include <linux/user-return-notifier.h> > #include <linux/dmi.h> > +#include <linux/kasan.h> > #include <linux/utsname.h> > #include <linux/stackprotector.h> > #include <linux/tick.h> > @@ -514,7 +515,7 @@ unsigned long arch_randomize_brk(struct mm_struct *mm) > */ > unsigned long get_wchan(struct task_struct *p) > { > - unsigned long start, bottom, top, sp, fp, ip; > + unsigned long start, bottom, top, sp, fp, ip, ret = 0; > int count = 0; > > if (!p || p == current || p->state == TASK_RUNNING) > @@ -550,14 +551,21 @@ unsigned long get_wchan(struct task_struct *p) > if (sp < bottom || sp > top) > return 0; > > + kasan_disable_current(); > fp = READ_ONCE(*(unsigned long *)sp); > do { > if (fp < bottom || fp > top) > - return 0; > + goto out; a break would do just fine too. > + > ip = READ_ONCE(*(unsigned long *)(fp + sizeof(unsigned long))); > - if (!in_sched_functions(ip)) > - return ip; > + if (!in_sched_functions(ip)) { > + ret = ip; > + goto out; ditto. > + } > fp = READ_ONCE(*(unsigned long *)fp); > } while (count++ < 16 && p->state != TASK_RUNNING); > - return 0; > + > +out: and then the label would not be needed. > + kasan_enable_current(); > + return ret; But that's all pretty disgusting really. Cannot we do better, such as annotating the function and then KASAN sorting out its false positives, or something like that? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists