From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5CC711FFC48; Wed, 24 Jun 2026 13:12:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782306734; cv=none; b=lDnAHd84yDf2fwiS23QiVsnozL0dNlOJV3C1WcbQvimjBnh3Tjjc2zuxdFMpP3mM9pW46CkpfJFGz78Ut2103+2wY2XOsTeyFIPGIzJbQqk1QLk4nDkFVLnZHc8PxsA3InRBsOY31Lo6cLRvyAfHIfIdM26rc1OSI33iJxPeoRM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782306734; c=relaxed/simple; bh=w0zMI2FXSw6dBKLZjPfmcO+nP7eNOr+LD0EOs1Hmil0=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=D3omK2g8XL93gznHvWa24XZGoM2D4c5v+jY80ipoSfL6416+ptoQWx9nKQ+y7uAMRAfoFailh3eq8wcamoryTiWIm29jKlWToRGYPM1U7CS7dPhgCnfXLggwLEYJ/2iMPm3v20CBUhoMznjYMG7VqW+SXwS89ooHys6gRRrQ2yA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KlcUi3ha; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KlcUi3ha" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1DB471F000E9; Wed, 24 Jun 2026 13:12:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782306733; bh=aSfCNoOHA4MMj4vfov5LjVgdAFVWxiZap6xazmrZ5TI=; h=From:Date:Subject:References:In-Reply-To:To:Cc; b=KlcUi3ha8THBZBaxRlkaw4IHC8/8+yagYeJXTxwtrNo82QHgjrci1MgjRIaPalwQ1 lqx4+ivcOwoC88q2iH56N9MKrtDPLroQhrv5lKYW0ic9siXsMT4SOAKuzo3wcfjKx9 l5zmB8B7++/9SSyv8UW7EousOsINWJSYCLZ4leLiyLgIun8qzpDvSCCWzTxxwk4Yh0 RxnZLg4D0O/8aY/BcoK4gr/rC39ySO/OauBj36v/ILp5Vq9K5fXGVeJtpojDhFR3ii y/n3UupI//rjsFoY1pgkjJX36ncIht4pyNl/yUps7blp9sgX6MRCA42DB36dXrhNt0 NSImtg6ZgDrIA== From: "Harry Yoo (Oracle)" Date: Wed, 24 Jun 2026 22:11:39 +0900 Subject: [PATCH RFC 2/4] mm/slab: handle allow_spin in slab_free_hook() instead of open coding Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20260624-kmalloc-nolock-fixes-v1-2-fdf4d17351dd@kernel.org> References: <20260624-kmalloc-nolock-fixes-v1-0-fdf4d17351dd@kernel.org> In-Reply-To: <20260624-kmalloc-nolock-fixes-v1-0-fdf4d17351dd@kernel.org> To: Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , Vlastimil Babka , Hao Li , Christoph Lameter , David Rientjes , Alexei Starovoitov , Pedro Falcato Cc: cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org X-Mailer: b4 0.14.3 kfree_nolock() deliberately skips slab free hooks that should not be called in unknown context. However, there is one more place that needs the same thing: memcg_alloc_abort_single(). Instead of open coding slab_free_hook(), let it handle the allow_spin parameter. This is required to correctly handle allocation failures due to memcg. There are two functional changes from this. First, memcg_alloc_abort_single() -> slab_free_hook() path no longer calls free hooks that acquire spinlocks if !allow_spin. Second, kfree_nolock() now follows init_on_free which has been ignored. Besides that, no functional change intended. Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().") Cc: stable@vger.kernel.org Signed-off-by: Harry Yoo (Oracle) --- mm/slub.c | 100 +++++++++++++++++++++++++++++++------------------------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index 32672a92581b..85760c8ff2e2 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2611,41 +2611,52 @@ struct rcu_delayed_free { * - memcg_slab_post_alloc_hook() * - alloc_tagging_slab_alloc_hook() * + * slab_free_hook() assumes that memcg_slab_free_hook() and + * alloc_tagging_slab_free_hook() are already invoked. + * * Free hooks that must handle missing corresponding alloc hooks: * - kmemleak_free_recursive() * - kfence_free() * - * Free hooks that have no alloc hook counterpart, and thus safe to call: + * Free hooks that have no alloc hook counterpart, and thus safe to call + * when spinning is allowed: * - debug_check_no_locks_freed() * - debug_check_no_obj_freed() * - __kcsan_check_access() */ static __always_inline bool slab_free_hook(struct kmem_cache *s, void *x, bool init, - bool after_rcu_delay) + bool after_rcu_delay, bool allow_spin) { /* Are the object contents still accessible? */ bool still_accessible = (s->flags & SLAB_TYPESAFE_BY_RCU) && !after_rcu_delay; - kmemleak_free_recursive(x, s->flags); kmsan_slab_free(s, x); - debug_check_no_locks_freed(x, s->object_size); - - if (!(s->flags & SLAB_DEBUG_OBJECTS)) - debug_check_no_obj_freed(x, s->object_size); - - /* Use KCSAN to help debug racy use-after-free. */ - if (!still_accessible) - __kcsan_check_access(x, s->object_size, - KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT); - - if (kfence_free(x)) - return false; + if (likely(allow_spin)) { + /* These hooks acquire spinlocks */ + kmemleak_free_recursive(x, s->flags); + debug_check_no_locks_freed(x, s->object_size); + if (!(s->flags & SLAB_DEBUG_OBJECTS)) + debug_check_no_obj_freed(x, s->object_size); + /* Use KCSAN to help debug racy use-after-free. */ + if (!still_accessible) + __kcsan_check_access(x, s->object_size, + KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT); + if (kfence_free(x)) + return false; + } /* * Give KASAN a chance to notice an invalid free operation before we * modify the object. + * + * If KASAN finds a bug it will do kasan_report_invalid_free() which + * will call raw_spin_lock_irqsave() which is technically unsafe from + * NMI, but take chance and report kernel bug. The sequence of + * kasan_report_invalid_free() -> raw_spin_lock_irqsave() -> NMI + * -> kfree_nolock() -> kasan_report_invalid_free() on the same CPU + * is double buggy and deserves to deadlock. */ if (kasan_slab_pre_free(s, x)) return false; @@ -2654,6 +2665,7 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init, if (still_accessible) { struct rcu_delayed_free *delayed_free; + VM_WARN_ON_ONCE(!allow_spin); delayed_free = kmalloc_obj(*delayed_free, GFP_NOWAIT); if (delayed_free) { /* @@ -2701,8 +2713,11 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init, set_orig_size(s, x, orig_size); } - /* KASAN might put x into memory quarantine, delaying its reuse. */ - return !kasan_slab_free(s, x, init, still_accessible, false); + /* + * KASAN might put x into memory quarantine, delaying its reuse. + * Skip quarantine when spinning is not allowed as it uses a spinlock. + */ + return !kasan_slab_free(s, x, init, still_accessible, !allow_spin); } static __fastpath_inline @@ -2714,9 +2729,10 @@ bool slab_free_freelist_hook(struct kmem_cache *s, void **head, void **tail, void *next = *head; void *old_tail = *tail; bool init; + bool allow_spin = true; if (is_kfence_address(next)) { - slab_free_hook(s, next, false, false); + slab_free_hook(s, next, false, false, allow_spin); return false; } @@ -2731,7 +2747,7 @@ bool slab_free_freelist_hook(struct kmem_cache *s, void **head, void **tail, next = get_freepointer(s, object); /* If object's reuse doesn't have to be delayed */ - if (likely(slab_free_hook(s, object, init, false))) { + if (likely(slab_free_hook(s, object, init, false, allow_spin))) { /* Move object to the new freelist */ set_freepointer(s, object, *head); *head = object; @@ -2954,7 +2970,7 @@ static bool __rcu_free_sheaf_prepare(struct kmem_cache *s, memcg_slab_free_hook(s, slab, p + i, 1, allow_spin); alloc_tagging_slab_free_hook(s, slab, p + i, 1); - if (unlikely(!slab_free_hook(s, p[i], init, true))) { + if (unlikely(!slab_free_hook(s, p[i], init, true, allow_spin))) { p[i] = p[--sheaf->size]; continue; } @@ -6225,7 +6241,7 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p) memcg_slab_free_hook(s, slab, p + i, 1, allow_spin); alloc_tagging_slab_free_hook(s, slab, p + i, 1); - if (unlikely(!slab_free_hook(s, p[i], init, false))) { + if (unlikely(!slab_free_hook(s, p[i], init, false, allow_spin))) { p[i] = p[--size]; continue; } @@ -6401,11 +6417,12 @@ void slab_free(struct kmem_cache *s, struct slab *slab, void *object, unsigned long addr) { bool allow_spin = true; + bool init = slab_want_init_on_free(s); memcg_slab_free_hook(s, slab, &object, 1, allow_spin); alloc_tagging_slab_free_hook(s, slab, &object, 1); - if (unlikely(!slab_free_hook(s, object, slab_want_init_on_free(s), false))) + if (unlikely(!slab_free_hook(s, object, init, false, allow_spin))) return; if (likely(can_free_to_pcs(slab)) && @@ -6422,10 +6439,12 @@ static noinline void memcg_alloc_abort_single(struct kmem_cache *s, void *object) { struct slab *slab = virt_to_slab(object); + bool init = slab_want_init_on_free(s); + bool allow_spin = true; alloc_tagging_slab_free_hook(s, slab, &object, 1); - if (likely(slab_free_hook(s, object, slab_want_init_on_free(s), false))) + if (likely(slab_free_hook(s, object, init, false, allow_spin))) __slab_free(s, slab, object, object, 1, _RET_IP_); } #endif @@ -6456,6 +6475,8 @@ static void slab_free_after_rcu_debug(struct rcu_head *rcu_head) void *object = delayed_free->object; struct slab *slab = virt_to_slab(object); struct kmem_cache *s; + bool allow_spin = true; + bool init; kfree(delayed_free); @@ -6469,8 +6490,9 @@ static void slab_free_after_rcu_debug(struct rcu_head *rcu_head) if (WARN_ON(!(s->flags & SLAB_TYPESAFE_BY_RCU))) return; + init = slab_want_init_on_free(s); /* resume freeing */ - if (slab_free_hook(s, object, slab_want_init_on_free(s), true)) { + if (slab_free_hook(s, object, init, true, allow_spin)) { __slab_free(s, slab, object, object, 1, _THIS_IP_); stat(s, FREE_SLOWPATH); } @@ -6742,6 +6764,7 @@ void kfree_nolock(const void *object) struct kmem_cache *s; void *x = (void *)object; bool allow_spin = false; + bool init; if (unlikely(ZERO_OR_NULL_PTR(object))) return; @@ -6756,33 +6779,10 @@ void kfree_nolock(const void *object) memcg_slab_free_hook(s, slab, &x, 1, allow_spin); alloc_tagging_slab_free_hook(s, slab, &x, 1); - /* - * Unlike slab_free() do NOT call the following: - * kmemleak_free_recursive(x, s->flags); - * debug_check_no_locks_freed(x, s->object_size); - * debug_check_no_obj_freed(x, s->object_size); - * __kcsan_check_access(x, s->object_size, ..); - * kfence_free(x); - * since they take spinlocks or not safe from any context. - */ - kmsan_slab_free(s, x); - /* - * If KASAN finds a kernel bug it will do kasan_report_invalid_free() - * which will call raw_spin_lock_irqsave() which is technically - * unsafe from NMI, but take chance and report kernel bug. - * The sequence of - * kasan_report_invalid_free() -> raw_spin_lock_irqsave() -> NMI - * -> kfree_nolock() -> kasan_report_invalid_free() on the same CPU - * is double buggy and deserves to deadlock. - */ - if (kasan_slab_pre_free(s, x)) + + init = slab_want_init_on_free(s); + if (!slab_free_hook(s, x, init, false, allow_spin)) return; - /* - * memcg, kasan_slab_pre_free are done for 'x'. - * The only thing left is kasan_poison without quarantine, - * since kasan quarantine takes locks and not supported from NMI. - */ - kasan_slab_free(s, x, false, false, /* skip quarantine */true); if (likely(can_free_to_pcs(slab)) && likely(free_to_pcs(s, x, allow_spin))) -- 2.53.0