* [PATCH v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab
@ 2025-10-15 12:59 Hao Ge
2025-10-15 13:11 ` Vlastimil Babka
2025-10-15 18:47 ` [syzbot ci] " syzbot ci
0 siblings, 2 replies; 4+ messages in thread
From: Hao Ge @ 2025-10-15 12:59 UTC (permalink / raw)
To: Vlastimil Babka, Andrew Morton, Christoph Lameter,
David Rientjes, Roman Gushchin, Harry Yoo
Cc: Alexei Starovoitov, Shakeel Butt, Suren Baghdasaryan, linux-mm,
linux-kernel, Hao Ge
From: Hao Ge <gehao@kylinos.cn>
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL,
But we did not clear it when freeing the slab. Since OBJEXTS_ALLOC_FAIL and
MEMCG_DATA_OBJEXTS currently share the same bit position, during the
release of the associated folio, a VM_BUG_ON_FOLIO() check in
folio_memcg_kmem() is triggered because it was mistakenly assumed that
a valid folio->memcg_data was not cleared before freeing the folio.
When freeing a slab, we clear slab->obj_exts if the obj_ext array has been
successfully allocated. So let's clear OBJEXTS_ALLOC_FAIL when freeing
a slab if the obj_ext array allocated fail to allow them to be returned
to the buddy system more smoothly.
Fixes: 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by: Harry Yoo <harry.yoo@oracle.com>
Signed-off-by: Hao Ge <gehao@kylinos.cn>
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
---
v4: Based on the discussion between Vlastimil and Harry,
modify the solution to clear OBJEXTS_ALLOC_FAIL when freeing a slab.
This does seem more reasonable. Thank you both.
---
mm/slab.h | 26 ++++++++++++++++++++++++++
mm/slub.c | 6 ++++++
2 files changed, 32 insertions(+)
diff --git a/mm/slab.h b/mm/slab.h
index 078daecc7cf5..52424d6871bd 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -547,6 +547,28 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
return (struct slabobj_ext *)(obj_exts & ~OBJEXTS_FLAGS_MASK);
}
+/*
+ * objexts_clear_alloc_fail - Clear the OBJEXTS_ALLOC_FAIL for
+ * the slab object extension vector associated with a slab.
+ * @slab: a pointer to the slab struct
+ */
+static inline void objexts_clear_alloc_fail(struct slab *slab)
+{
+ unsigned long obj_exts = READ_ONCE(slab->obj_exts);
+
+#ifdef CONFIG_MEMCG
+ /*
+ * obj_exts should be either NULL, a valid pointer with
+ * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
+ */
+ VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
+ obj_exts != OBJEXTS_ALLOC_FAIL, slab_page(slab));
+ VM_BUG_ON_PAGE(obj_exts & MEMCG_DATA_KMEM, slab_page(slab));
+#endif
+
+ obj_exts &= ~OBJEXTS_ALLOC_FAIL;
+ WRITE_ONCE(slab->obj_exts, obj_exts);
+}
int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
gfp_t gfp, bool new_slab);
@@ -557,6 +579,10 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
return NULL;
}
+static inline void objexts_clear_alloc_fail(struct slab *slab)
+{
+}
+
#endif /* CONFIG_SLAB_OBJ_EXT */
static inline enum node_stat_item cache_vmstat_idx(struct kmem_cache *s)
diff --git a/mm/slub.c b/mm/slub.c
index b1f15598fbfd..80166a4a62f9 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2169,6 +2169,12 @@ static inline void free_slab_obj_exts(struct slab *slab)
{
struct slabobj_ext *obj_exts;
+ /*
+ * If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL,
+ * Therefore, we should clear the OBJEXTS_ALLOC_FAIL flag first when freeing a slab.
+ */
+ objexts_clear_alloc_fail(slab);
+
obj_exts = slab_obj_exts(slab);
if (!obj_exts)
return;
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab
2025-10-15 12:59 [PATCH v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab Hao Ge
@ 2025-10-15 13:11 ` Vlastimil Babka
2025-10-15 13:37 ` Hao Ge
2025-10-15 18:47 ` [syzbot ci] " syzbot ci
1 sibling, 1 reply; 4+ messages in thread
From: Vlastimil Babka @ 2025-10-15 13:11 UTC (permalink / raw)
To: Hao Ge, Andrew Morton, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo
Cc: Alexei Starovoitov, Shakeel Butt, Suren Baghdasaryan, linux-mm,
linux-kernel, Hao Ge
On 10/15/25 14:59, Hao Ge wrote:
> From: Hao Ge <gehao@kylinos.cn>
>
> If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL,
> But we did not clear it when freeing the slab. Since OBJEXTS_ALLOC_FAIL and
> MEMCG_DATA_OBJEXTS currently share the same bit position, during the
> release of the associated folio, a VM_BUG_ON_FOLIO() check in
> folio_memcg_kmem() is triggered because it was mistakenly assumed that
> a valid folio->memcg_data was not cleared before freeing the folio.
>
> When freeing a slab, we clear slab->obj_exts if the obj_ext array has been
> successfully allocated. So let's clear OBJEXTS_ALLOC_FAIL when freeing
> a slab if the obj_ext array allocated fail to allow them to be returned
> to the buddy system more smoothly.
Thanks!
> Fixes: 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
> Suggested-by: Harry Yoo <harry.yoo@oracle.com>
> Signed-off-by: Hao Ge <gehao@kylinos.cn>
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Since we changed the approach completely, I think we should not carry over
the previously added review tags in this case.
> ---
> v4: Based on the discussion between Vlastimil and Harry,
> modify the solution to clear OBJEXTS_ALLOC_FAIL when freeing a slab.
> This does seem more reasonable. Thank you both.
> ---
> mm/slab.h | 26 ++++++++++++++++++++++++++
> mm/slub.c | 6 ++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/mm/slab.h b/mm/slab.h
> index 078daecc7cf5..52424d6871bd 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -547,6 +547,28 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
> return (struct slabobj_ext *)(obj_exts & ~OBJEXTS_FLAGS_MASK);
> }
>
> +/*
> + * objexts_clear_alloc_fail - Clear the OBJEXTS_ALLOC_FAIL for
> + * the slab object extension vector associated with a slab.
> + * @slab: a pointer to the slab struct
> + */
> +static inline void objexts_clear_alloc_fail(struct slab *slab)
> +{
> + unsigned long obj_exts = READ_ONCE(slab->obj_exts);
> +
> +#ifdef CONFIG_MEMCG
> + /*
> + * obj_exts should be either NULL, a valid pointer with
> + * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
> + */
> + VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
> + obj_exts != OBJEXTS_ALLOC_FAIL, slab_page(slab));
> + VM_BUG_ON_PAGE(obj_exts & MEMCG_DATA_KMEM, slab_page(slab));
> +#endif
> +
> + obj_exts &= ~OBJEXTS_ALLOC_FAIL;
> + WRITE_ONCE(slab->obj_exts, obj_exts);
> +}
This is much larger than necessary I think. See below.
> int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> gfp_t gfp, bool new_slab);
>
> @@ -557,6 +579,10 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
> return NULL;
> }
>
> +static inline void objexts_clear_alloc_fail(struct slab *slab)
> +{
> +}
> +
> #endif /* CONFIG_SLAB_OBJ_EXT */
>
> static inline enum node_stat_item cache_vmstat_idx(struct kmem_cache *s)
> diff --git a/mm/slub.c b/mm/slub.c
> index b1f15598fbfd..80166a4a62f9 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2169,6 +2169,12 @@ static inline void free_slab_obj_exts(struct slab *slab)
> {
> struct slabobj_ext *obj_exts;
>
> + /*
> + * If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL,
> + * Therefore, we should clear the OBJEXTS_ALLOC_FAIL flag first when freeing a slab.
> + */
> + objexts_clear_alloc_fail(slab);
> +
> obj_exts = slab_obj_exts(slab);
> if (!obj_exts)
> return;
It should be enough that this return path does "slab->obj_exts = 0;" no?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab
2025-10-15 13:11 ` Vlastimil Babka
@ 2025-10-15 13:37 ` Hao Ge
0 siblings, 0 replies; 4+ messages in thread
From: Hao Ge @ 2025-10-15 13:37 UTC (permalink / raw)
To: Vlastimil Babka, Andrew Morton, Christoph Lameter,
David Rientjes, Roman Gushchin, Harry Yoo
Cc: Alexei Starovoitov, Shakeel Butt, Suren Baghdasaryan, linux-mm,
linux-kernel, Hao Ge
Hi Vlastimil
On 2025/10/15 21:11, Vlastimil Babka wrote:
> On 10/15/25 14:59, Hao Ge wrote:
>> From: Hao Ge <gehao@kylinos.cn>
>>
>> If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL,
>> But we did not clear it when freeing the slab. Since OBJEXTS_ALLOC_FAIL and
>> MEMCG_DATA_OBJEXTS currently share the same bit position, during the
>> release of the associated folio, a VM_BUG_ON_FOLIO() check in
>> folio_memcg_kmem() is triggered because it was mistakenly assumed that
>> a valid folio->memcg_data was not cleared before freeing the folio.
>>
>> When freeing a slab, we clear slab->obj_exts if the obj_ext array has been
>> successfully allocated. So let's clear OBJEXTS_ALLOC_FAIL when freeing
>> a slab if the obj_ext array allocated fail to allow them to be returned
>> to the buddy system more smoothly.
> Thanks!
>
>> Fixes: 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
>> Suggested-by: Harry Yoo <harry.yoo@oracle.com>
>> Signed-off-by: Hao Ge <gehao@kylinos.cn>
>> Reviewed-by: Suren Baghdasaryan <surenb@google.com>
>> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Since we changed the approach completely, I think we should not carry over
> the previously added review tags in this case.
got it.
>
>> ---
>> v4: Based on the discussion between Vlastimil and Harry,
>> modify the solution to clear OBJEXTS_ALLOC_FAIL when freeing a slab.
>> This does seem more reasonable. Thank you both.
>> ---
>> mm/slab.h | 26 ++++++++++++++++++++++++++
>> mm/slub.c | 6 ++++++
>> 2 files changed, 32 insertions(+)
>>
>> diff --git a/mm/slab.h b/mm/slab.h
>> index 078daecc7cf5..52424d6871bd 100644
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -547,6 +547,28 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
>> return (struct slabobj_ext *)(obj_exts & ~OBJEXTS_FLAGS_MASK);
>> }
>>
>> +/*
>> + * objexts_clear_alloc_fail - Clear the OBJEXTS_ALLOC_FAIL for
>> + * the slab object extension vector associated with a slab.
>> + * @slab: a pointer to the slab struct
>> + */
>> +static inline void objexts_clear_alloc_fail(struct slab *slab)
>> +{
>> + unsigned long obj_exts = READ_ONCE(slab->obj_exts);
>> +
>> +#ifdef CONFIG_MEMCG
>> + /*
>> + * obj_exts should be either NULL, a valid pointer with
>> + * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
>> + */
>> + VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
>> + obj_exts != OBJEXTS_ALLOC_FAIL, slab_page(slab));
>> + VM_BUG_ON_PAGE(obj_exts & MEMCG_DATA_KMEM, slab_page(slab));
>> +#endif
>> +
>> + obj_exts &= ~OBJEXTS_ALLOC_FAIL;
>> + WRITE_ONCE(slab->obj_exts, obj_exts);
>> +}
> This is much larger than necessary I think. See below.
>
>> int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
>> gfp_t gfp, bool new_slab);
>>
>> @@ -557,6 +579,10 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
>> return NULL;
>> }
>>
>> +static inline void objexts_clear_alloc_fail(struct slab *slab)
>> +{
>> +}
>> +
>> #endif /* CONFIG_SLAB_OBJ_EXT */
>>
>> static inline enum node_stat_item cache_vmstat_idx(struct kmem_cache *s)
>> diff --git a/mm/slub.c b/mm/slub.c
>> index b1f15598fbfd..80166a4a62f9 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -2169,6 +2169,12 @@ static inline void free_slab_obj_exts(struct slab *slab)
>> {
>> struct slabobj_ext *obj_exts;
>>
>> + /*
>> + * If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL,
>> + * Therefore, we should clear the OBJEXTS_ALLOC_FAIL flag first when freeing a slab.
>> + */
>> + objexts_clear_alloc_fail(slab);
>> +
>> obj_exts = slab_obj_exts(slab);
>> if (!obj_exts)
>> return;
> It should be enough that this return path does "slab->obj_exts = 0;" no?
I might have had a momentary mental block just now,sorry,
this modification method is indeed much simpler.
I will send v5 as soon as possible.
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [syzbot ci] Re: slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab
2025-10-15 12:59 [PATCH v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab Hao Ge
2025-10-15 13:11 ` Vlastimil Babka
@ 2025-10-15 18:47 ` syzbot ci
1 sibling, 0 replies; 4+ messages in thread
From: syzbot ci @ 2025-10-15 18:47 UTC (permalink / raw)
To: akpm, ast, cl, gehao, hao.ge, harry.yoo, linux-kernel, linux-mm,
rientjes, roman.gushchin, shakeel.butt, surenb, vbabka
Cc: syzbot, syzkaller-bugs
syzbot ci has tested the following series
[v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab
https://lore.kernel.org/all/20251015125945.481950-1-hao.ge@linux.dev
* [PATCH v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab
and found the following issue:
kernel BUG in __free_slab
Full report is available here:
https://ci.syzbot.org/series/61ff4fe1-2e84-410d-ad85-42ead772d9c8
***
kernel BUG in __free_slab
tree: torvalds
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux
base: 3a8660878839faadb4f1a6dd72c3179c1df56787
arch: amd64
compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
config: https://ci.syzbot.org/builds/e5875084-ea86-418c-979d-5b00cded86ca/config
syz repro: https://ci.syzbot.org/findings/1f8913d8-6d9a-448a-9419-90b758c82c3b/syz_repro
___sys_sendmsg+0x21f/0x2a0 net/socket.c:2684
__sys_sendmsg net/socket.c:2716 [inline]
__do_sys_sendmsg net/socket.c:2721 [inline]
__se_sys_sendmsg net/socket.c:2719 [inline]
__x64_sys_sendmsg+0x19b/0x260 net/socket.c:2719
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xfa/0xfa0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
------------[ cut here ]------------
kernel BUG at mm/slab.h:544!
Oops: invalid opcode: 0000 [#1] SMP KASAN PTI
CPU: 0 UID: 0 PID: 15 Comm: ksoftirqd/0 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:slab_obj_exts mm/slab.h:543 [inline]
RIP: 0010:free_slab_obj_exts mm/slub.c:2178 [inline]
RIP: 0010:unaccount_slab mm/slub.c:3186 [inline]
RIP: 0010:__free_slab+0x1b8/0x1e0 mm/slub.c:3286
Code: e8 2d 8a 0c ff 90 0f 0b 48 89 df 48 c7 c6 18 ad 91 8d e8 1b 8a 0c ff 90 0f 0b 48 89 df 48 c7 c6 9d 66 95 8d e8 09 8a 0c ff 90 <0f> 0b 48 89 df 48 c7 c6 18 ad 91 8d e8 f7 89 0c ff 90 0f 0b 48 89
RSP: 0000:ffffc900001478b0 EFLAGS: 00010246
RAX: 680205e52e87ff00 RBX: ffffea00044c3c80 RCX: 680205e52e87ff00
RDX: 0000000000000000 RSI: ffffffff8d7e835a RDI: ffffffff8bc076e0
RBP: 0000000000000001 R08: ffffffff8f9e1177 R09: 1ffffffff1f3c22e
R10: dffffc0000000000 R11: fffffbfff1f3c22f R12: ffffffff821b61b0
R13: ffffffff81a82877 R14: ffff888160415a00 R15: ffffea00044c3c98
FS: 0000000000000000(0000) GS:ffff88818e70c000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fef42bb12f8 CR3: 000000000df38000 CR4: 00000000000006f0
Call Trace:
<TASK>
rcu_do_batch kernel/rcu/tree.c:2605 [inline]
rcu_core+0xcab/0x1770 kernel/rcu/tree.c:2861
handle_softirqs+0x286/0x870 kernel/softirq.c:622
run_ksoftirqd+0x9b/0x100 kernel/softirq.c:1063
smpboot_thread_fn+0x542/0xa60 kernel/smpboot.c:160
kthread+0x711/0x8a0 kernel/kthread.c:463
ret_from_fork+0x4bc/0x870 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:slab_obj_exts mm/slab.h:543 [inline]
RIP: 0010:free_slab_obj_exts mm/slub.c:2178 [inline]
RIP: 0010:unaccount_slab mm/slub.c:3186 [inline]
RIP: 0010:__free_slab+0x1b8/0x1e0 mm/slub.c:3286
Code: e8 2d 8a 0c ff 90 0f 0b 48 89 df 48 c7 c6 18 ad 91 8d e8 1b 8a 0c ff 90 0f 0b 48 89 df 48 c7 c6 9d 66 95 8d e8 09 8a 0c ff 90 <0f> 0b 48 89 df 48 c7 c6 18 ad 91 8d e8 f7 89 0c ff 90 0f 0b 48 89
RSP: 0000:ffffc900001478b0 EFLAGS: 00010246
RAX: 680205e52e87ff00 RBX: ffffea00044c3c80 RCX: 680205e52e87ff00
RDX: 0000000000000000 RSI: ffffffff8d7e835a RDI: ffffffff8bc076e0
RBP: 0000000000000001 R08: ffffffff8f9e1177 R09: 1ffffffff1f3c22e
R10: dffffc0000000000 R11: fffffbfff1f3c22f R12: ffffffff821b61b0
R13: ffffffff81a82877 R14: ffff888160415a00 R15: ffffea00044c3c98
FS: 0000000000000000(0000) GS:ffff88818e70c000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fef42bb12f8 CR3: 000000000df38000 CR4: 00000000000006f0
***
If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
Tested-by: syzbot@syzkaller.appspotmail.com
---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-15 18:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-15 12:59 [PATCH v4] slab: clear OBJEXTS_ALLOC_FAIL when freeing a slab Hao Ge
2025-10-15 13:11 ` Vlastimil Babka
2025-10-15 13:37 ` Hao Ge
2025-10-15 18:47 ` [syzbot ci] " syzbot ci
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox