lkml.org 
[lkml]   [2022]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 16/43] kmsan: mm: call KMSAN hooks from SLUB code
From
On 12/14/21 17:20, Alexander Potapenko wrote:
> In order to report uninitialized memory coming from heap allocations
> KMSAN has to poison them unless they're created with __GFP_ZERO.
>
> It's handy that we need KMSAN hooks in the places where
> init_on_alloc/init_on_free initialization is performed.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
> Link: https://linux-review.googlesource.com/id/I6954b386c5c5d7f99f48bb6cbcc74b75136ce86e
> ---
> mm/slab.h | 1 +
> mm/slub.c | 26 +++++++++++++++++++++++---
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/mm/slab.h b/mm/slab.h
> index 56ad7eea3ddfb..6175a74047b47 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -521,6 +521,7 @@ static inline void slab_post_alloc_hook(struct kmem_cache *s,
> memset(p[i], 0, s->object_size);
> kmemleak_alloc_recursive(p[i], s->object_size, 1,
> s->flags, flags);
> + kmsan_slab_alloc(s, p[i], flags);
> }
>
> memcg_slab_post_alloc_hook(s, objcg, flags, size, p);
> diff --git a/mm/slub.c b/mm/slub.c
> index abe7db581d686..5a63486e52531 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -22,6 +22,7 @@
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> #include <linux/kasan.h>
> +#include <linux/kmsan.h>
> #include <linux/cpu.h>
> #include <linux/cpuset.h>
> #include <linux/mempolicy.h>
> @@ -346,10 +347,13 @@ static inline void *freelist_dereference(const struct kmem_cache *s,
> (unsigned long)ptr_addr);
> }
>
> +/*
> + * See the comment to get_freepointer_safe().
> + */

I did...

> static inline void *get_freepointer(struct kmem_cache *s, void *object)
> {
> object = kasan_reset_tag(object);
> - return freelist_dereference(s, object + s->offset);
> + return kmsan_init(freelist_dereference(s, object + s->offset));

... but I don't see why it applies to get_freepointer() too? What am I missing?

> }
>
> static void prefetch_freepointer(const struct kmem_cache *s, void *object)
> @@ -357,18 +361,28 @@ static void prefetch_freepointer(const struct kmem_cache *s, void *object)
> prefetchw(object + s->offset);
> }
>
> +/*
> + * When running under KMSAN, get_freepointer_safe() may return an uninitialized
> + * pointer value in the case the current thread loses the race for the next
> + * memory chunk in the freelist. In that case this_cpu_cmpxchg_double() in
> + * slab_alloc_node() will fail, so the uninitialized value won't be used, but
> + * KMSAN will still check all arguments of cmpxchg because of imperfect
> + * handling of inline assembly.
> + * To work around this problem, use kmsan_init() to force initialize the
> + * return value of get_freepointer_safe().
> + */
> static inline void *get_freepointer_safe(struct kmem_cache *s, void *object)
> {
> unsigned long freepointer_addr;
> void *p;
>
> if (!debug_pagealloc_enabled_static())
> - return get_freepointer(s, object);
> + return kmsan_init(get_freepointer(s, object));

So here kmsan_init() is done twice?

>
> object = kasan_reset_tag(object);
> freepointer_addr = (unsigned long)object + s->offset;
> copy_from_kernel_nofault(&p, (void **)freepointer_addr, sizeof(p));
> - return freelist_ptr(s, p, freepointer_addr);
> + return kmsan_init(freelist_ptr(s, p, freepointer_addr));
> }
>
> static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp)

\
 
 \ /
  Last update: 2022-01-07 18:23    [W:0.338 / U:1.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site