From: Imre Kaloz <kaloz@kernel.org>
To: Harry Yoo <harry@kernel.org>
Cc: Vlastimil Babka <vbabka@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Hao Li <hao.li@linux.dev>, Christoph Lameter <cl@gentwo.org>,
David Rientjes <rientjes@google.com>,
Roman Gushchin <roman.gushchin@linux.dev>,
Jann Horn <jannh@google.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
Date: Thu, 1 Oct 2026 17:42:36 +0200 (CEST) [thread overview]
Message-ID: <aef97b97-0bd7-d720-7f59-8c0078747370@kernel.org> (raw)
In-Reply-To: <ar5pESlkEMqNsgxv@thinkstation>
Hi Harry,
On Thu, 1 Oct 2026, Harry Yoo wrote:
> Hi Imre,
> Thanks for catching and fixing this!
>
> On Wed, Sep 30, 2026 at 09:51:09PM +0200, Imre Kaloz wrote:
>> kmem_cache_destroy() tests slab_state >= FULL for sysfs_slab_unlink()
>> and again in kmem_cache_release() for sysfs_slab_release(), with the
>> cache already off slab_caches in between. If slab_late_init() runs in
>> that gap it sets slab_state to FULL but never calls sysfs_slab_add() for
>> the unlinked cache, so kmem_cache_release() ends up in kobject_put() on
>> a kobject that was never initialized:
>>
>> WARNING: lib/kobject.c:734 at kobject_put+0x64/0x2c0, CPU#1: kworker/u8:3/55
>> kobject: '(null)' ((____ptrval____)): is not initialized, yet kobject_put() is being called.
>> refcount_t: underflow; use-after-free.
>> kobject_put+0x64/0x2c0
>> sysfs_slab_release+0xc/0x20
>> kmem_cache_destroy+0x104/0x1e0
>> bioset_exit+0x13c/0x1e0
>> disk_release+0x54/0x140
>> put_disk+0x18/0x40
>> floppy_async_init+0xbec/0xd10
>>
>> Seen on sparc64 at boot, where the asynchronous floppy init tears down
>> its bio slab while the late initcalls are running.
>>
>> Read slab_state once, under slab_mutex which slab_late_init() holds when
>> it sets FULL, and use the result for both the unlink and the release.
>> The kobject flags (state_initialized, state_in_sysfs) were considered as
>> the key instead, but slab_state is what cache creation and
>> slab_late_init() decide on.
>>
>> debugfs_slab_release() is not part of the race: it only looks the cache
>> up by name in the debugfs root and does nothing before that root exists.
>>
>> Fixes: 4ec10268ed98 ("mm, slab: unlink slabinfo, sysfs and debugfs immediately")
>
> Overall looks good to me, but could you please explain why it's
> not relevant before this commit? pre-4ec10268 still reads slab_state
> outside slab_mutex.
>
The outside-mutex read is older, yes. What 4ec10268ed98 changed is that
unlink and release no longer share it.
Before that commit, kmem_cache_release() did one test and used it for
both:
if (slab_state >= FULL) {
sysfs_slab_unlink(s);
sysfs_slab_release(s);
} else {
slab_kmem_cache_release(s);
}
So the two could not disagree. 4ec10268 moved sysfs_slab_unlink() into
kmem_cache_destroy(), after list_del() and after dropping slab_mutex,
and left a second slab_state test in kmem_cache_release() for
sysfs_slab_release(). The cache is already off slab_caches in between.
That is the window in the warning: the first test sees < FULL, so the
kobject is never linked; slab_late_init() then sets FULL under
slab_mutex and calls sysfs_slab_add() only for caches still on the
list; the second test sees FULL and kobject_put()s a kobject that
kobject_init() never ran on.
A single read cannot produce that split decision, which is why Fixes:
points at 4ec10268ed98.
There is a related older window: after list_del() and before that
single read, slab_sysfs_init() could set FULL, skip this cache, and
the single read would then call both unlink and release on an
uninitialized kobject. I have not hit that, and this patch does not
close it.
Best,
Imre
next prev parent reply other threads:[~2026-10-01 15:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-30 19:51 [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy() Imre Kaloz
2026-09-30 21:26 ` Jose A. Perez de Azpillaga
2026-10-01 14:22 ` Harry Yoo
2026-10-01 15:42 ` Imre Kaloz [this message]
2026-10-13 12:38 ` Harry Yoo
2026-10-13 12:51 ` Imre Kaloz
2026-10-13 14:29 ` Harry Yoo
2026-10-13 16:32 ` Imre Kaloz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aef97b97-0bd7-d720-7f59-8c0078747370@kernel.org \
--to=kaloz@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=cl@gentwo.org \
--cc=hao.li@linux.dev \
--cc=harry@kernel.org \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=rientjes@google.com \
--cc=roman.gushchin@linux.dev \
--cc=stable@vger.kernel.org \
--cc=vbabka@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.