diff mbox series

[RFC,v3,13/36] kmsan: make READ_ONCE_TASK_STACK() return initialized values

Message ID 20191122112621.204798-14-glider@google.com (mailing list archive)
State New, archived
Headers show
Series Add KernelMemorySanitizer infrastructure | expand

Commit Message

Alexander Potapenko Nov. 22, 2019, 11:25 a.m. UTC
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: linux-mm@kvack.org
---

Change-Id: Ie73e5a41fdc8195699928e65f5cbe0d3d3c9e2fa
---
 arch/x86/include/asm/unwind.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Marco Elver Dec. 2, 2019, 10:07 a.m. UTC | #1
On Fri, 22 Nov 2019 at 12:27, <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: linux-mm@kvack.org

Acked-by: Marco Elver <elver@google.com>

assuming previous patch's include for kmsan-checks.h is added.

> ---
>
> Change-Id: Ie73e5a41fdc8195699928e65f5cbe0d3d3c9e2fa
> ---
>  arch/x86/include/asm/unwind.h | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
> index 499578f7e6d7..f60c2bd1ddf2 100644
> --- a/arch/x86/include/asm/unwind.h
> +++ b/arch/x86/include/asm/unwind.h
> @@ -100,9 +100,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 +112,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.24.0.432.g9d3f5f5b63-goog
>
Alexander Potapenko Dec. 5, 2019, 3:52 p.m. UTC | #2
On Mon, Dec 2, 2019 at 11:08 AM Marco Elver <elver@google.com> wrote:
>
> On Fri, 22 Nov 2019 at 12:27, <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: linux-mm@kvack.org
>
> Acked-by: Marco Elver <elver@google.com>
>
> assuming previous patch's include for kmsan-checks.h is added.
Yes, I'll add that include here as well in v4.
> > ---
> >
> > Change-Id: Ie73e5a41fdc8195699928e65f5cbe0d3d3c9e2fa
> > ---
> >  arch/x86/include/asm/unwind.h | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
> > index 499578f7e6d7..f60c2bd1ddf2 100644
> > --- a/arch/x86/include/asm/unwind.h
> > +++ b/arch/x86/include/asm/unwind.h
> > @@ -100,9 +100,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 +112,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.24.0.432.g9d3f5f5b63-goog
> >
diff mbox series

Patch

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 499578f7e6d7..f60c2bd1ddf2 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -100,9 +100,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 +112,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)