| Message ID | 20200325161249.55095-12-glider@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add KernelMemorySanitizer infrastructure | expand |
On Wed, Mar 25, 2020 at 5:13 PM <glider@google.com> wrote: > > To avoid false positives, assume that reading from the task stack > always produces initialized values. > > Signed-off-by: Alexander Potapenko <glider@google.com> > To: Alexander Potapenko <glider@google.com> > Cc: Vegard Nossum <vegard.nossum@oracle.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Marco Elver <elver@google.com> > Cc: Andrey Konovalov <andreyknvl@google.com> > Cc: linux-mm@kvack.org > Acked-by: Marco Elver <elver@google.com> Reviewed-by: Andrey Konovalov <andreyknvl@google.com> > > --- > v4: > - added an #include as requested by Marco Elver > > Change-Id: Ie73e5a41fdc8195699928e65f5cbe0d3d3c9e2fa > --- > arch/x86/include/asm/unwind.h | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h > index 499578f7e6d7b..82c3bceb9999c 100644 > --- a/arch/x86/include/asm/unwind.h > +++ b/arch/x86/include/asm/unwind.h > @@ -4,6 +4,7 @@ > > #include <linux/sched.h> > #include <linux/ftrace.h> > +#include <linux/kmsan-checks.h> > #include <asm/ptrace.h> > #include <asm/stacktrace.h> > > @@ -100,9 +101,10 @@ void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size, > #endif > > /* > - * This disables KASAN checking when reading a value from another task's stack, > - * since the other task could be running on another CPU and could have poisoned > - * the stack in the meantime. > + * This disables KASAN/KMSAN checking when reading a value from another task's > + * stack, since the other task could be running on another CPU and could have > + * poisoned the stack in the meantime. Frame pointers are uninitialized by > + * default, so for KMSAN we mark the return value initialized unconditionally. > */ > #define READ_ONCE_TASK_STACK(task, x) \ > ({ \ > @@ -111,7 +113,7 @@ void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size, > val = READ_ONCE(x); \ > else \ > val = READ_ONCE_NOCHECK(x); \ > - val; \ > + KMSAN_INIT_VALUE(val); \ > }) > > static inline bool task_on_another_cpu(struct task_struct *task) > -- > 2.25.1.696.g5e7596f4ac-goog >
On Thu, Apr 23, 2020 at 9:15 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > On Wed, Mar 25, 2020 at 5:13 PM <glider@google.com> wrote: > > > > To avoid false positives, assume that reading from the task stack > > always produces initialized values. > > > > Signed-off-by: Alexander Potapenko <glider@google.com> > > To: Alexander Potapenko <glider@google.com> > > Cc: Vegard Nossum <vegard.nossum@oracle.com> > > Cc: Dmitry Vyukov <dvyukov@google.com> > > Cc: Marco Elver <elver@google.com> > > Cc: Andrey Konovalov <andreyknvl@google.com> > > Cc: linux-mm@kvack.org > > Acked-by: Marco Elver <elver@google.com> > > Reviewed-by: Andrey Konovalov <andreyknvl@google.com> On the second though, nack. > > > > > --- > > v4: > > - added an #include as requested by Marco Elver > > > > Change-Id: Ie73e5a41fdc8195699928e65f5cbe0d3d3c9e2fa > > --- > > arch/x86/include/asm/unwind.h | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h > > index 499578f7e6d7b..82c3bceb9999c 100644 > > --- a/arch/x86/include/asm/unwind.h > > +++ b/arch/x86/include/asm/unwind.h > > @@ -4,6 +4,7 @@ > > > > #include <linux/sched.h> > > #include <linux/ftrace.h> > > +#include <linux/kmsan-checks.h> > > #include <asm/ptrace.h> > > #include <asm/stacktrace.h> > > > > @@ -100,9 +101,10 @@ void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size, > > #endif > > > > /* > > - * This disables KASAN checking when reading a value from another task's stack, > > - * since the other task could be running on another CPU and could have poisoned > > - * the stack in the meantime. > > + * This disables KASAN/KMSAN checking when reading a value from another task's > > + * stack, since the other task could be running on another CPU and could have > > + * poisoned the stack in the meantime. Frame pointers are uninitialized by > > + * default, so for KMSAN we mark the return value initialized unconditionally. > > */ > > #define READ_ONCE_TASK_STACK(task, x) \ > > ({ \ > > @@ -111,7 +113,7 @@ void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size, > > val = READ_ONCE(x); \ > > else \ > > val = READ_ONCE_NOCHECK(x); \ > > - val; \ > > + KMSAN_INIT_VALUE(val); \ If the task != current, we use READ_ONCE_TASK_STACK(), which already does KMSAN_INIT_VALUE(), so no need for it here? > > }) > > > > static inline bool task_on_another_cpu(struct task_struct *task) > > -- > > 2.25.1.696.g5e7596f4ac-goog > >
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h index 499578f7e6d7b..82c3bceb9999c 100644 --- a/arch/x86/include/asm/unwind.h +++ b/arch/x86/include/asm/unwind.h @@ -4,6 +4,7 @@ #include <linux/sched.h> #include <linux/ftrace.h> +#include <linux/kmsan-checks.h> #include <asm/ptrace.h> #include <asm/stacktrace.h> @@ -100,9 +101,10 @@ void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size, #endif /* - * This disables KASAN checking when reading a value from another task's stack, - * since the other task could be running on another CPU and could have poisoned - * the stack in the meantime. + * This disables KASAN/KMSAN checking when reading a value from another task's + * stack, since the other task could be running on another CPU and could have + * poisoned the stack in the meantime. Frame pointers are uninitialized by + * default, so for KMSAN we mark the return value initialized unconditionally. */ #define READ_ONCE_TASK_STACK(task, x) \ ({ \ @@ -111,7 +113,7 @@ void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size, val = READ_ONCE(x); \ else \ val = READ_ONCE_NOCHECK(x); \ - val; \ + KMSAN_INIT_VALUE(val); \ }) static inline bool task_on_another_cpu(struct task_struct *task)