mm/slab: do not init any kfence objects on allocation

When init (zeroing) on allocation is requested, for kmalloc() we
generally have to zero the full object size even if a smaller size is
requested, in order to provide krealloc()'s __GFP_ZERO guarantees.

When we end up allocating a kfence object, kfence performs the zeroing
on its own because it has its own redzone beyond the requested size.
Thus slab_post_alloc_hook() has an 'init' parameter which has to be
evaluated in all callers (via slab_want_init_on_alloc()) and should be
false for kfence allocations.

For kfence allocations in slab_alloc_node() this is achieved by subtly
skipping over the slab_want_init_on_alloc() call. Other callers (i.e.
kmem_cache_alloc_bulk_noprof()) however evaluate it unconditionally even
if they do end up with a kfence allocation. This is only subtly not a
problem, as those are not kmalloc allocations and thus the "requested
size" equals s->object_size and thus it cannot interfere with kfence's
redzone. There's just a unnecessary double zeroing (in both kfence and
slab_post_alloc_hook()), but it's all very fragile and contradicts the
comment in kfence_guarded_alloc().

Remove this subtlety and simplify the code by eliminating the init
parameter from slab_post_alloc_hook() and make it call
slab_want_init_on_alloc() itself. Instead add a is_kfence_address()
check before performing the memset, which will start doing the right
thing for all callers of slab_post_alloc_hook().

This potentially adds overhead of the is_kfence_address() check to
allocation hotpath, but that one is designed to be as small as possible,
and it's only evaluated if zeroing is about to happen. This means (aside
from init_on_alloc hardening) only for __GFP_ZERO allocations, and the
zeroing itself comes with an overhead likely larger than the added
check.

While at it, refactor the handling of evaluating when KASAN does the
init instead of SLUB, with no intended functional changes. A
non-functional change is that we don't pass kasan_init as true to
kasan_slab_alloc() if kasan has no integrated init, but then the value
is ignored anyway, so it's theoretically more correct.

Thanks to Harry Yoo for the initial refactoring attempt, and for updated
comments that are used here.

Link: https://patch.msgid.link/20260610-slab_alloc_flags-v2-2-7190909db118@kernel.org
Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Hao Li <hao.li@linux.dev>
Reviewed-by: Marco Elver <elver@google.com>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 655dc5ce32409e47b469fcd6088174b23ea7a67f..5e0b406924e97c219e47da8cf0173ecfb045af77 100644 (file)
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -500,7 +500,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g
 
        /*
         * We check slab_want_init_on_alloc() ourselves, rather than letting
-        * SL*B do the initialization, as otherwise we might overwrite KFENCE's
+        * slab do the initialization, as otherwise it might overwrite KFENCE's
         * redzone.
         */
        if (unlikely(slab_want_init_on_alloc(gfp, cache)))

diff --git a/mm/slub.c b/mm/slub.c
index e2ee8f1aaccf384db5e919a097d3f9795fe566cf..d762cbe5d0407071a52ac274b2aaddf69fd6aadf 100644 (file)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4565,13 +4565,13 @@ struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s, gfp_t flags)
 
 static __fastpath_inline
 bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
-                         gfp_t flags, size_t size, void **p, bool init,
+                         gfp_t flags, size_t size, void **p,
                          unsigned int orig_size)
 {
+       bool init = slab_want_init_on_alloc(flags, s);
        unsigned int zero_size = s->object_size;
-       bool kasan_init = init;
-       size_t i;
        gfp_t init_flags = flags & gfp_allowed_mask;
+       bool kasan_init = false;
 
        /*
         * For kmalloc object, the allocated size (object_size) can be larger
@@ -4588,28 +4588,33 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
                zero_size = orig_size;
 
        /*
-        * When slab_debug is enabled, avoid memory initialization integrated
-        * into KASAN and instead zero out the memory via the memset below with
-        * the proper size. Otherwise, KASAN might overwrite SLUB redzones and
-        * cause false-positive reports. This does not lead to a performance
+        * ARM64 can set memory tags and zero the memory using a single
+        * instruction. Since HW_TAGS KASAN uses that while tagging the object,
+        * separate zeroing is unnecessary.
+        *
+        * However, KASAN never zeroes memory when slab_debug is enabled to
+        * avoid overwriting SLUB redzones. This does not lead to a performance
         * penalty on production builds, as slab_debug is not intended to be
         * enabled there.
         */
-       if (__slub_debug_enabled())
-               kasan_init = false;
+       if (kasan_has_integrated_init() && !__slub_debug_enabled()) {
+               kasan_init = init;
+               init = false;
+       }
 
-       /*
-        * As memory initialization might be integrated into KASAN,
-        * kasan_slab_alloc and initialization memset must be
-        * kept together to avoid discrepancies in behavior.
-        *
-        * As p[i] might get tagged, memset and kmemleak hook come after KASAN.
-        */
-       for (i = 0; i < size; i++) {
+       for (size_t i = 0; i < size; i++) {
                p[i] = kasan_slab_alloc(s, p[i], init_flags, kasan_init);
-               if (p[i] && init && (!kasan_init ||
-                                    !kasan_has_integrated_init()))
+
+               /*
+                * memset and hooks come after KASAN as p[i] might get tagged
+                *
+                * kfence zeroes the object instead of SLUB to avoid overwriting
+                * its own redzone starting at orig_size, which could happen
+                * with SLUB zeroing full s->object_size
+                */
+               if (init && p[i] && !is_kfence_address(p[i]))
                        memset(p[i], 0, zero_size);
+
                if (gfpflags_allow_spinning(flags))
                        kmemleak_alloc_recursive(p[i], s->object_size, 1,
                                                 s->flags, init_flags);
@@ -4910,7 +4915,6 @@ static __fastpath_inline void *slab_alloc_node(struct kmem_cache *s, struct list
                gfp_t gfpflags, int node, unsigned long addr, size_t orig_size)
 {
        void *object;
-       bool init = false;
 
        s = slab_pre_alloc_hook(s, gfpflags);
        if (unlikely(!s))
@@ -4926,16 +4930,13 @@ static __fastpath_inline void *slab_alloc_node(struct kmem_cache *s, struct list
                object = __slab_alloc_node(s, gfpflags, node, addr, orig_size);
 
        maybe_wipe_obj_freeptr(s, object);
-       init = slab_want_init_on_alloc(gfpflags, s);
 
 out:
        /*
-        * When init equals 'true', like for kzalloc() family, only
-        * @orig_size bytes might be zeroed instead of s->object_size
         * In case this fails due to memcg_slab_post_alloc_hook(),
         * object is set to NULL
         */
-       slab_post_alloc_hook(s, lru, gfpflags, 1, &object, init, orig_size);
+       slab_post_alloc_hook(s, lru, gfpflags, 1, &object, orig_size);
 
        return object;
 }
@@ -5230,7 +5231,6 @@ kmem_cache_alloc_from_sheaf_noprof(struct kmem_cache *s, gfp_t gfp,
                                   struct slab_sheaf *sheaf)
 {
        void *ret = NULL;
-       bool init;
 
        if (sheaf->size == 0)
                goto out;
@@ -5240,10 +5240,8 @@ kmem_cache_alloc_from_sheaf_noprof(struct kmem_cache *s, gfp_t gfp,
        if (likely(!ret))
                ret = sheaf->objects[--sheaf->size];
 
-       init = slab_want_init_on_alloc(gfp, s);
-
        /* add __GFP_NOFAIL to force successful memcg charging */
-       slab_post_alloc_hook(s, NULL, gfp | __GFP_NOFAIL, 1, &ret, init, s->object_size);
+       slab_post_alloc_hook(s, NULL, gfp | __GFP_NOFAIL, 1, &ret, s->object_size);
 out:
        trace_kmem_cache_alloc(_RET_IP_, ret, s, gfp, NUMA_NO_NODE);
 
@@ -5423,8 +5421,7 @@ retry:
 
 success:
        maybe_wipe_obj_freeptr(s, ret);
-       slab_post_alloc_hook(s, NULL, alloc_gfp, 1, &ret,
-                            slab_want_init_on_alloc(alloc_gfp, s), orig_size);
+       slab_post_alloc_hook(s, NULL, alloc_gfp, 1, &ret, orig_size);
 
        ret = kasan_kmalloc(s, ret, orig_size, alloc_gfp);
        return ret;
@@ -7339,8 +7336,7 @@ bool kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags,
 
 out:
        /* memcg and kmem_cache debug support and memory initialization */
-       return likely(slab_post_alloc_hook(s, NULL, flags, size, p,
-                       slab_want_init_on_alloc(flags, s), s->object_size));
+       return likely(slab_post_alloc_hook(s, NULL, flags, size, p, s->object_size));
 }
 EXPORT_SYMBOL(kmem_cache_alloc_bulk_noprof);