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

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

[Harry Yoo (Oracle)]

[+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@vger.kernel.org>
> Reported-by: https://sashiko.dev/#/patchset/20260415143735.2974230-1-elver%40google.com
> Signed-off-by: Marco Elver <elver@google.com>
> ---

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)

-- 
Cheers,
Harry / Hyeonggon

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

[Marco Elver]

On Fri, 17 Apr 2026 at 06:42, Harry Yoo (Oracle) <harry@kernel.org> 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@vger.kernel.org>
> > Reported-by: https://sashiko.dev/#/patchset/20260415143735.2974230-1-elver%40google.com
> > Signed-off-by: Marco Elver <elver@google.com>
> > ---
>
> 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
(it's also not confidence-inspiring that vrealloc_node_align_noprof
has a bunch of TODOs sprinkled all over...).
Since you found that, do you want to claim it?

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

Thanks,
-- Marco

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@kernel.org> 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@vger.kernel.org>
> > > Reported-by: https://sashiko.dev/#/patchset/20260415143735.2974230-1-elver%40google.com
> > > Signed-off-by: Marco Elver <elver@google.com>
> > > ---
> >
> > 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@kernel.org>

> (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@zohomail.in

> 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