| Message ID | 20240729141928.4545a093@imladris.surriel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm,slub: do not call do_slab_free for kfence object | expand |
On 7/29/24 2:19 PM, Rik van Riel wrote: > In 782f8906f805 the freeing of kfence objects was moved from deep > inside do_free_slab to the wrapper functions outside. This is a nice > change, but unfortunately it missed one spot in __kmem_cache_free_bulk. > > This results in a crash like this: > > BUG skbuff_head_cache (Tainted: G S B E ): Padding overwritten. 0xffff88907fea0f00-0xffff88907fea0fff @offset=3840 > > slab_err (mm/slub.c:1129) > free_to_partial_list (mm/slub.c:? mm/slub.c:4036) > slab_pad_check (mm/slub.c:864 mm/slub.c:1290) > check_slab (mm/slub.c:?) > free_to_partial_list (mm/slub.c:3171 mm/slub.c:4036) > kmem_cache_alloc_bulk (mm/slub.c:? mm/slub.c:4495 mm/slub.c:4586 mm/slub.c:4635) > napi_build_skb (net/core/skbuff.c:348 net/core/skbuff.c:527 net/core/skbuff.c:549) > > All the other callers to do_free_slab appear to be ok. > > Add a kfence_free check in __kmem_cache_free_bulk to avoid the crash. > > Reported-by: Chris Mason <clm@meta.com> > Fixes: 782f8906f805 ("mm/slub: free KFENCE objects in slab_free_hook()") > Cc: stable@kernel.org > Signed-off-by: Rik van Riel <riel@surriel.com> We found this after bisecting a slab corruption down to the kfence patch, and with this patch applied we're no longer falling over. So thanks Rik! -chris
On 7/29/24 8:46 PM, Chris Mason wrote: > > > On 7/29/24 2:19 PM, Rik van Riel wrote: >> In 782f8906f805 the freeing of kfence objects was moved from deep >> inside do_free_slab to the wrapper functions outside. This is a nice >> change, but unfortunately it missed one spot in __kmem_cache_free_bulk. >> >> This results in a crash like this: >> >> BUG skbuff_head_cache (Tainted: G S B E ): Padding overwritten. 0xffff88907fea0f00-0xffff88907fea0fff @offset=3840 >> >> slab_err (mm/slub.c:1129) >> free_to_partial_list (mm/slub.c:? mm/slub.c:4036) >> slab_pad_check (mm/slub.c:864 mm/slub.c:1290) >> check_slab (mm/slub.c:?) >> free_to_partial_list (mm/slub.c:3171 mm/slub.c:4036) >> kmem_cache_alloc_bulk (mm/slub.c:? mm/slub.c:4495 mm/slub.c:4586 mm/slub.c:4635) >> napi_build_skb (net/core/skbuff.c:348 net/core/skbuff.c:527 net/core/skbuff.c:549) >> >> All the other callers to do_free_slab appear to be ok. changed do_free_slab to do_slab_free in two places. >> >> Add a kfence_free check in __kmem_cache_free_bulk to avoid the crash. >> >> Reported-by: Chris Mason <clm@meta.com> >> Fixes: 782f8906f805 ("mm/slub: free KFENCE objects in slab_free_hook()") >> Cc: stable@kernel.org >> Signed-off-by: Rik van Riel <riel@surriel.com> > > We found this after bisecting a slab corruption down to the kfence > patch, and with this patch applied we're no longer falling over. So > thanks Rik! Indeed thanks and sorry for the trouble! Given that __kmem_cache_free_bulk is currently only used to unwind a kmem_cache_bulk_alloc() that runs out of memory in the middle of the operation, I'm surprised you saw this happen reliably enough to bisect it. Added to slab/for-6.11-rc1/fixes > -chris
On 7/30/24 6:01 AM, Vlastimil Babka wrote: > On 7/29/24 8:46 PM, Chris Mason wrote: >> >> >> On 7/29/24 2:19 PM, Rik van Riel wrote: >>> Reported-by: Chris Mason <clm@meta.com> >>> Fixes: 782f8906f805 ("mm/slub: free KFENCE objects in slab_free_hook()") >>> Cc: stable@kernel.org >>> Signed-off-by: Rik van Riel <riel@surriel.com> >> >> We found this after bisecting a slab corruption down to the kfence >> patch, and with this patch applied we're no longer falling over. So >> thanks Rik! > > Indeed thanks and sorry for the trouble! Given that > __kmem_cache_free_bulk is currently only used to unwind a > kmem_cache_bulk_alloc() that runs out of memory in the middle of the > operation, I'm surprised you saw this happen reliably enough to bisect it. > The repro was just forcing two sequential OOMs during iperf load on top of mlx5 ethernet: Test machine: - iperf -s -V Load generator: - iperf -c test_machine -P 10 -w 1k -l 1k -V --time 900 Test machine: - hog all memory until OOM - Do it one more time Since we didn't have memory corruptions on other nics, I was pretty sure the bisect had gone wrong when all the remaining commits were in MM. Nothing against our friends in networking, but MM bugs are usually easier to fix, so I was pretty happy after confirming kfence as the cause. > Added to slab/for-6.11-rc1/fixes Thanks! -chris
diff --git a/mm/slub.c b/mm/slub.c index 3520acaf9afa..c9d8a2497fd6 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4690,6 +4690,9 @@ static void __kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) if (!df.slab) continue; + if (kfence_free(df.freelist)) + continue; + do_slab_free(df.s, df.slab, df.freelist, df.tail, df.cnt, _RET_IP_); } while (likely(size));
In 782f8906f805 the freeing of kfence objects was moved from deep inside do_free_slab to the wrapper functions outside. This is a nice change, but unfortunately it missed one spot in __kmem_cache_free_bulk. This results in a crash like this: BUG skbuff_head_cache (Tainted: G S B E ): Padding overwritten. 0xffff88907fea0f00-0xffff88907fea0fff @offset=3840 slab_err (mm/slub.c:1129) free_to_partial_list (mm/slub.c:? mm/slub.c:4036) slab_pad_check (mm/slub.c:864 mm/slub.c:1290) check_slab (mm/slub.c:?) free_to_partial_list (mm/slub.c:3171 mm/slub.c:4036) kmem_cache_alloc_bulk (mm/slub.c:? mm/slub.c:4495 mm/slub.c:4586 mm/slub.c:4635) napi_build_skb (net/core/skbuff.c:348 net/core/skbuff.c:527 net/core/skbuff.c:549) All the other callers to do_free_slab appear to be ok. Add a kfence_free check in __kmem_cache_free_bulk to avoid the crash. Reported-by: Chris Mason <clm@meta.com> Fixes: 782f8906f805 ("mm/slub: free KFENCE objects in slab_free_hook()") Cc: stable@kernel.org Signed-off-by: Rik van Riel <riel@surriel.com> --- mm/slub.c | 3 +++ 1 file changed, 3 insertions(+)