Re: [PATCH bpf-next 1/4] slab: Introduce kmem_cache_alloc_nolock()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Deadlock in NMI context due to synchronous freeing on memcg charging failure
--

commit 831b8a379d6dfbde2570286ba8453174bc236695
Author: Alexei Starovoitov <ast@kernel.org>

slab: Introduce kmem_cache_alloc_nolock()

This commit introduces kmem_cache_alloc_nolock_noprof() and its wrapper 
kmem_cache_alloc_nolock(). It is designed to allocate an object from a
specific cache and aims to be safe to call from any context, including NMI
and IRQ-off environments.

> diff --git a/mm/slub.c b/mm/slub.c
> index a2bf3756ca7d0..601986aaebdfc 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
[ ... ]
> @@ -5402,6 +5402,45 @@ void *kmalloc_nolock_noprof(size_t size, gfp_t gfp_flags, int node)
>  }
>  EXPORT_SYMBOL_GPL(kmalloc_nolock_noprof);
>  
> +/**
> + * kmem_cache_alloc_nolock - Allocate one object from a specific cache,
> + * safe from any context (including NMI/IRQ-off), like kmalloc_nolock().
> + *
> + * Returns NULL on failure (including the trylock paths that may transiently
> + * fail under contention).
> + */
> +void *kmem_cache_alloc_nolock_noprof(struct kmem_cache *s, gfp_t gfp_flags,
> +				     int node)
> +{
> +	gfp_t alloc_gfp = __GFP_NOWARN | __GFP_NOMEMALLOC | gfp_flags;
> +	void *ret;
> +
> +	VM_WARN_ON_ONCE(gfp_flags & ~(__GFP_ACCOUNT | __GFP_ZERO |
> +				      __GFP_NO_OBJ_EXT));
[ ... ]
> +	maybe_wipe_obj_freeptr(s, ret);
> +	slab_post_alloc_hook(s, NULL, alloc_gfp, 1, &ret,
> +			     slab_want_init_on_alloc(alloc_gfp, s),
> +			     s->object_size);

[Severity: Critical]
Does this sequence risk a hard deadlock if called from NMI context?

The function permits __GFP_ACCOUNT, but if memory cgroup charging fails
inside memcg_slab_post_alloc_hook(), it synchronously aborts the allocation:

mm/slub.c:memcg_slab_post_alloc_hook() {
    ...
    if (memcg_alloc_charge_slab(s, alloc_gfp, lru, p, size)) {
        for (i = 0; i < size; i++)
            memcg_alloc_abort_single(s, p[i]);
    ...
}

This calls memcg_alloc_abort_single(), which immediately attempts to free
the object:

mm/slub.c:memcg_alloc_abort_single() {
    ...
    if (likely(slab_free_hook(s, object, slab_want_init_on_free(s), false)))
        __slab_free(s, slab, object, object, 1, _RET_IP_);
    ...
}

Both slab_free_hook() (via kmemleak and kfence hooks) and __slab_free()
will attempt to acquire non-NMI-safe spinlocks like n->list_lock. If the
interrupted context was holding these locks, wouldn't this synchronous free
cause a deadlock, breaking the "safe from any context" guarantee?

> +	return kasan_kmalloc(s, ret, s->object_size, alloc_gfp);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529202438.16408-1-alexei.starovoitov@gmail.com?part=1