On Thu, Apr 23, 2020 at 9:15 PM Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote:
>
> On Wed, Mar 25, 2020 at 5:13 PM <glider@xxxxxxxxxx> wrote:
> >
> > To avoid false positives, assume that reading from the task stack
> > always produces initialized values.
> >
> > Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx>
> > To: Alexander Potapenko <glider@xxxxxxxxxx>
> > Cc: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
> > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> > Cc: Marco Elver <elver@xxxxxxxxxx>
> > Cc: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> > Cc: linux-mm@xxxxxxxxx
> > Acked-by: Marco Elver <elver@xxxxxxxxxx>
>
> Reviewed-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
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
> >
