* [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
@ 2026-09-30 19:51 Imre Kaloz
2026-09-30 21:26 ` Jose A. Perez de Azpillaga
2026-10-01 14:22 ` Harry Yoo
0 siblings, 2 replies; 8+ messages in thread
From: Imre Kaloz @ 2026-09-30 19:51 UTC (permalink / raw)
To: Vlastimil Babka, Harry Yoo, Andrew Morton
Cc: Hao Li, Christoph Lameter, David Rientjes, Roman Gushchin,
Jann Horn, linux-mm, linux-kernel, stable
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")
Cc: stable@vger.kernel.org
Signed-off-by: Imre Kaloz <kaloz@kernel.org>
---
mm/slab_common.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 7223a7596dab..de11edfd1a0b 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -515,10 +515,10 @@ EXPORT_SYMBOL(kmem_buckets_create);
* and release of the kobject does not need slab_mutex or cpu_hotplug_lock
* protection. So they are now done without holding those locks.
*/
-static void kmem_cache_release(struct kmem_cache *s)
+static void kmem_cache_release(struct kmem_cache *s, bool sysfs_ready)
{
kfence_shutdown_cache(s);
- if (__is_defined(SLAB_SUPPORTS_SYSFS) && slab_state >= FULL)
+ if (__is_defined(SLAB_SUPPORTS_SYSFS) && sysfs_ready)
sysfs_slab_release(s);
else
slab_kmem_cache_release(s);
@@ -533,6 +533,7 @@ void slab_kmem_cache_release(struct kmem_cache *s)
void kmem_cache_destroy(struct kmem_cache *s)
{
+ bool sysfs_ready;
int err;
if (unlikely(!s) || !kasan_check_byte(s))
@@ -580,10 +581,18 @@ void kmem_cache_destroy(struct kmem_cache *s)
list_del(&s->list);
+ /*
+ * slab_late_init() sets slab_state to FULL under slab_mutex and adds
+ * sysfs entries only for caches still on the list. Sample the state
+ * here, so that a cache unlinked before that point is not handed to
+ * sysfs_slab_release() with an uninitialized kobject.
+ */
+ sysfs_ready = slab_state >= FULL;
+
mutex_unlock(&slab_mutex);
cpus_read_unlock();
- if (slab_state >= FULL)
+ if (sysfs_ready)
sysfs_slab_unlink(s);
debugfs_slab_release(s);
@@ -593,7 +602,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
if (s->flags & SLAB_TYPESAFE_BY_RCU)
rcu_barrier();
- kmem_cache_release(s);
+ kmem_cache_release(s, sysfs_ready);
}
EXPORT_SYMBOL(kmem_cache_destroy);
base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
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
1 sibling, 0 replies; 8+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-09-30 21:26 UTC (permalink / raw)
To: Imre Kaloz
Cc: Vlastimil Babka, Harry Yoo, Andrew Morton, Hao Li,
Christoph Lameter, David Rientjes, Roman Gushchin, Jann Horn,
linux-mm, linux-kernel, stable
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.
the enum has nothing between UP and FULL, and the create path skips
sysfs_slab_add() while slab_state <= UP, so both tests are the one that
decides whether the kobject exists, and a false value cannot skip a
kobject_put() that was owed.
it would read better to me if the changelog said that. otherwise LGTM.
Reviewed-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
> 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")
> Cc: stable@vger.kernel.org
> Signed-off-by: Imre Kaloz <kaloz@kernel.org>
--
cheers,
jose a. p-a
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
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
1 sibling, 1 reply; 8+ messages in thread
From: Harry Yoo @ 2026-10-01 14:22 UTC (permalink / raw)
To: Imre Kaloz
Cc: Vlastimil Babka, Andrew Morton, Hao Li, Christoph Lameter,
David Rientjes, Roman Gushchin, Jann Horn, linux-mm, linux-kernel,
stable
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.
> Cc: stable@vger.kernel.org
> Signed-off-by: Imre Kaloz <kaloz@kernel.org>
--
Cheers,
Harry / Hyeonggon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
2026-10-01 14:22 ` Harry Yoo
@ 2026-10-01 15:42 ` Imre Kaloz
2026-10-13 12:38 ` Harry Yoo
0 siblings, 1 reply; 8+ messages in thread
From: Imre Kaloz @ 2026-10-01 15:42 UTC (permalink / raw)
To: Harry Yoo
Cc: Vlastimil Babka, Andrew Morton, Hao Li, Christoph Lameter,
David Rientjes, Roman Gushchin, Jann Horn, linux-mm, linux-kernel,
stable
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
2026-10-01 15:42 ` Imre Kaloz
@ 2026-10-13 12:38 ` Harry Yoo
2026-10-13 12:51 ` Imre Kaloz
0 siblings, 1 reply; 8+ messages in thread
From: Harry Yoo @ 2026-10-13 12:38 UTC (permalink / raw)
To: Imre Kaloz
Cc: Vlastimil Babka, Andrew Morton, Hao Li, Christoph Lameter,
David Rientjes, Roman Gushchin, Jann Horn, linux-mm, linux-kernel,
stable
Uh, apologies for the late reply.
Catching up after trip at LPC...
On Thu, Oct 01, 2026 at 05:42:36PM +0200, Imre Kaloz wrote:
> 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.
Right.
> 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.
Right.
> 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.
Right.
> 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.
Right.
> 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.
No, with this patch slab_sysfs_init() cannot set the state to FULL
because the slab_caches list and slab_state are protected by slab_mutex.
> I have not hit that, and this patch does not close it.
I think the patch does close two bugs.
One introduced by 4ec10268ed98 (reading slab_state twice),
another (reading slab_state outside slab_mutex) introduced by, hmm,
not sure which commit introduced it :) should be very old.
--
Cheers,
Harry / Hyeonggon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
2026-10-13 12:38 ` Harry Yoo
@ 2026-10-13 12:51 ` Imre Kaloz
2026-10-13 14:29 ` Harry Yoo
0 siblings, 1 reply; 8+ messages in thread
From: Imre Kaloz @ 2026-10-13 12:51 UTC (permalink / raw)
To: Harry Yoo
Cc: Vlastimil Babka, Andrew Morton, Hao Li, Christoph Lameter,
David Rientjes, Roman Gushchin, Jann Horn, linux-mm, linux-kernel,
stable
Hi Harry,
On Tue, 13 Oct 2026, Harry Yoo wrote:
> Uh, apologies for the late reply.
> Catching up after trip at LPC...
No worries, hope LPC was good!
> On Thu, Oct 01, 2026 at 05:42:36PM +0200, Imre Kaloz wrote:
<snip>
>> 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.
>
> No, with this patch slab_sysfs_init() cannot set the state to FULL
> because the slab_caches list and slab_state are protected by slab_mutex.
>
>> I have not hit that, and this patch does not close it.
>
> I think the patch does close two bugs.
>
> One introduced by 4ec10268ed98 (reading slab_state twice),
> another (reading slab_state outside slab_mutex) introduced by, hmm,
> not sure which commit introduced it :) should be very old.
You're right, I got that one wrong. Sampling under slab_mutex in the
same section as the list_del() means slab_late_init() either sees the
cache on the list or we see slab_state < FULL, so the older window is
closed too.
As for where the outside-mutex read came from, as far as I can tell it's
0495e337b703 ("mm/slab_common: Deleting kobject in kmem_cache_destroy()
without holding slab_mutex/cpu_hotplug_lock"). Before that, the unlink
and release and their slab_state checks ran in shutdown_cache() under
slab_mutex. SLAB_TYPESAFE_BY_RCU caches were already released from the
workfn outside the mutex since 657dc2f97220.
Want me to send a v2 with a second Fixes: for 0495e337b703, or is it
fine to add it when applying?
Best,
Imre
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
2026-10-13 12:51 ` Imre Kaloz
@ 2026-10-13 14:29 ` Harry Yoo
2026-10-13 16:32 ` Imre Kaloz
0 siblings, 1 reply; 8+ messages in thread
From: Harry Yoo @ 2026-10-13 14:29 UTC (permalink / raw)
To: Imre Kaloz
Cc: Vlastimil Babka, Andrew Morton, Hao Li, Christoph Lameter,
David Rientjes, Roman Gushchin, Jann Horn, linux-mm, linux-kernel,
stable
On Tue, Oct 13, 2026 at 02:51:30PM +0200, Imre Kaloz wrote:
> Hi Harry,
Hi Imre,
> On Tue, 13 Oct 2026, Harry Yoo wrote:
>
> > Uh, apologies for the late reply.
> > Catching up after trip at LPC...
>
> No worries, hope LPC was good!
Thanks. It was good.
> > On Thu, Oct 01, 2026 at 05:42:36PM +0200, Imre Kaloz wrote:
>
> <snip>
>
> > > 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.
> >
> > No, with this patch slab_sysfs_init() cannot set the state to FULL
> > because the slab_caches list and slab_state are protected by slab_mutex.
> >
> > > I have not hit that, and this patch does not close it.
> >
> > I think the patch does close two bugs.
> >
> > One introduced by 4ec10268ed98 (reading slab_state twice),
> > another (reading slab_state outside slab_mutex) introduced by, hmm,
> > not sure which commit introduced it :) should be very old.
>
> You're right, I got that one wrong. Sampling under slab_mutex in the
> same section as the list_del() means slab_late_init() either sees the
> cache on the list or we see slab_state < FULL, so the older window is
> closed too.
>
> As for where the outside-mutex read came from, as far as I can tell it's
> 0495e337b703 ("mm/slab_common: Deleting kobject in kmem_cache_destroy()
> without holding slab_mutex/cpu_hotplug_lock"). Before that, the unlink
> and release and their slab_state checks ran in shutdown_cache() under
> slab_mutex.
0495e337b703 Probably yes. Not 100% sure.
It's quite messy, reading slab_state has been done inside, switched
to outside, and back to inside, and so on.
But should be fine as long as we can find the first commit that
affects -stable kernels (>= 5.10.y)
> SLAB_TYPESAFE_BY_RCU caches were already released from the
> workfn outside the mutex since 657dc2f97220.
>
> Want me to send a v2 with a second Fixes: for 0495e337b703, or is it
> fine to add it when applying?
Could you please verify fixing which bugs resolves the issue on sparc64
described in the changelog? Reading it twice, or outside slab_mutex, or
both?
--
Cheers,
Harry / Hyeonggon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/slab: sample slab_state once in kmem_cache_destroy()
2026-10-13 14:29 ` Harry Yoo
@ 2026-10-13 16:32 ` Imre Kaloz
0 siblings, 0 replies; 8+ messages in thread
From: Imre Kaloz @ 2026-10-13 16:32 UTC (permalink / raw)
To: Harry Yoo
Cc: Vlastimil Babka, Andrew Morton, Hao Li, Christoph Lameter,
David Rientjes, Roman Gushchin, Jann Horn, linux-mm, linux-kernel,
stable
Hi Harry,
On Tue, 13 Oct 2026, Harry Yoo wrote:
<snip>
>>>> 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.
>>>
>>> No, with this patch slab_sysfs_init() cannot set the state to FULL
>>> because the slab_caches list and slab_state are protected by slab_mutex.
>>>
>>>> I have not hit that, and this patch does not close it.
>>>
>>> I think the patch does close two bugs.
>>>
>>> One introduced by 4ec10268ed98 (reading slab_state twice),
>>> another (reading slab_state outside slab_mutex) introduced by, hmm,
>>> not sure which commit introduced it :) should be very old.
>>
>> You're right, I got that one wrong. Sampling under slab_mutex in the
>> same section as the list_del() means slab_late_init() either sees the
>> cache on the list or we see slab_state < FULL, so the older window is
>> closed too.
>>
>> As for where the outside-mutex read came from, as far as I can tell it's
>> 0495e337b703 ("mm/slab_common: Deleting kobject in kmem_cache_destroy()
>> without holding slab_mutex/cpu_hotplug_lock"). Before that, the unlink
>> and release and their slab_state checks ran in shutdown_cache() under
>> slab_mutex.
>
> 0495e337b703 Probably yes. Not 100% sure.
>
> It's quite messy, reading slab_state has been done inside, switched
> to outside, and back to inside, and so on.
>
> But should be fine as long as we can find the first commit that
> affects -stable kernels (>= 5.10.y)
>
>> SLAB_TYPESAFE_BY_RCU caches were already released from the
>> workfn outside the mutex since 657dc2f97220.
>>
>> Want me to send a v2 with a second Fixes: for 0495e337b703, or is it
>> fine to add it when applying?
>
> Could you please verify fixing which bugs resolves the issue on sparc64
> described in the changelog? Reading it twice, or outside slab_mutex, or
> both?
I tested it. I built five variants of rc7 with a small test that
destroys a SLAB_TYPESAFE_BY_RCU cache while slab_late_init() runs
(same path the floppy bio slab takes), and booted each 10 times on
x86 and 8 times on sparc64. Boots that hit the WARN:
x86 sparc64
mainline 9/10 8/8
one read, in kmem_cache_release() 9/10 8/8
unlink check under slab_mutex,
release still re-reads 10/10 8/8
one read under slab_mutex (this patch) 0/10 0/8
one read right after mutex_unlock() 0/10 0/8
So it takes both. The read that hurts is the one at release time,
after rcu_barrier(); fixing only the double read keeps it, and so
does moving only the unlink check under the mutex. Reading once
right after the unlock didn't trigger it, but that is still racy,
just a much smaller window.
For stable, as far as I can tell the release-side read has been
outside slab_mutex since d50d82faa0c9 ("slub: fix failure when we
delete and create a slab cache") in v4.18, so that would cover
5.10.y. The patch doesn't apply before 6.12, so the older branches
need their own backports.
Best,
Imre
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-13 16:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox