Re: [PATCH] slub: fix data loss and overflow in krealloc()

[Harry Yoo (Oracle)]

On Fri, Apr 17, 2026 at 11:05:09AM +0200, Marco Elver wrote:
> On Fri, 17 Apr 2026 at 06:42, Harry Yoo (Oracle) <harry@xxxxxxxxxx> wrote:
> > [+Cc relevant folks]
> >
> > On Thu, Apr 16, 2026 at 03:25:07PM +0200, Marco Elver wrote:
> > > Commit 2cd8231796b5 ("mm/slub: allow to set node and align in
> > > k[v]realloc") introduced the ability to force a reallocation if the
> > > original object does not satisfy new alignment or NUMA node, even when
> > > the object is being shrunk.
> > >
> > > This introduced two bugs in the reallocation fallback path:
> > >
> > > 1. Data loss during NUMA migration: The jump to 'alloc_new' happens
> > >    before 'ks' and 'orig_size' are initialized. As a result, the
> > >    memcpy() in the 'alloc_new' block would copy 0 bytes into the new
> > >    allocation.
> >
> > Ouch.
> >
> > > 2. Buffer overflow during shrinking: When shrinking an object while
> > >    forcing a new alignment, 'new_size' is smaller than the old size.
> > >    However, the memcpy() used the old size ('orig_size ?: ks'), leading
> > >    to an out-of-bounds write.
> >
> > Right. before the commit we didn't reallocate when the size is smaller.
> >
> > > The same overflow bug exists in the kvrealloc() fallback path, where the
> > > old bucket size ksize(p) is copied into the new buffer without being
> > > bounded by the new size.
> > >
> > > A simple reproducer:
> > >
> > >       // e.g. add to lkdtm as KREALLOC_SHRINK_OVERFLOW
> > >       while (1) {
> > >               void *p = kmalloc(128, GFP_KERNEL);
> > >               p = krealloc_node_align(p, 64, 256, GFP_KERNEL, NUMA_NO_NODE);
> > >               kfree(p);
> > >       }
> > >
> > > demonstrates the issue:
> > >
> > >   ==================================================================
> > >   BUG: KFENCE: out-of-bounds write in memcpy_orig+0x68/0x130
> > >
> > >   Out-of-bounds write at 0xffff8883ad757038 (120B right of kfence-#47):
> > >    memcpy_orig+0x68/0x130
> > >    krealloc_node_align_noprof+0x1c8/0x340
> > >    lkdtm_KREALLOC_SHRINK_OVERFLOW+0x8c/0xc0 [lkdtm]
> > >    lkdtm_do_action+0x3a/0x60 [lkdtm]
> > >    ...
> > >
> > >   kfence-#47: 0xffff8883ad756fc0-0xffff8883ad756fff, size=64, cache=kmalloc-64
> > >
> > >   allocated by task 316 on cpu 7 at 97.680481s (0.021813s ago):
> > >    krealloc_node_align_noprof+0x19c/0x340
> > >    lkdtm_KREALLOC_SHRINK_OVERFLOW+0x8c/0xc0 [lkdtm]
> > >    lkdtm_do_action+0x3a/0x60 [lkdtm]
> > >    ...
> > >   ==================================================================
> > >
> > > Fix it by moving the old size calculation to the top of __do_krealloc()
> > > and bounding all copy lengths by the new allocation size.
> > >
> > > Fixes: 2cd8231796b5 ("mm/slub: allow to set node and align in k[v]realloc")
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > Reported-by: https://sashiko.dev/#/patchset/20260415143735.2974230-1-elver%40google.com
> > > Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
> > > ---
> >
> > Looks good to me, but I think we still have a similar issue in
> > vrealloc_node_align_noprof()? (goto need_realloc; due to NUMA mismatch
> > but the new size is smaller)
> 
> Good find.
>
> That's a separate patch, though, since it's in the vmalloc subsystem

You're right. for this patch:

Looks good to me,
Reviewed-by: Harry Yoo (Oracle) <harry@xxxxxxxxxx>

> (it's also not confidence-inspiring that vrealloc_node_align_noprof
> has a bunch of TODOs sprinkled all over...).

;)

looks like one of the TODOs is going to be tacked though. (shrinking)
https://lore.kernel.org/linux-mm/20260404-vmalloc-shrink-v10-0-335759165dfa@xxxxxxxxxxx

> Since you found that, do you want to claim it?

I have many stuffs going on my plate now (including re-reviewing the
typed kmalloc caches patch) so it'd be nice if somebody could claim :)

> Also by the looks of it slub and vmalloc patches go through different
> trees these days per MAINTAINERS.

Right. slab patches go through the slab tree.

-- 
Cheers,
Harry / Hyeonggon