Skip to content

Commit 082a6d0

Browse files
melverVlastimil Babka (SUSE)
authored andcommitted
slub: fix data loss and overflow in krealloc()
Commit 2cd8231 ("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. 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. 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: 2cd8231 ("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> Link: https://patch.msgid.link/20260416132837.3787694-1-elver@google.com Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
1 parent 43cfbdd commit 082a6d0

1 file changed

Lines changed: 12 additions & 12 deletions

File tree

mm/slub.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6645,16 +6645,6 @@ __do_krealloc(const void *p, size_t new_size, unsigned long align, gfp_t flags,
66456645
if (!kasan_check_byte(p))
66466646
return NULL;
66476647

6648-
/*
6649-
* If reallocation is not necessary (e. g. the new size is less
6650-
* than the current allocated size), the current allocation will be
6651-
* preserved unless __GFP_THISNODE is set. In the latter case a new
6652-
* allocation on the requested node will be attempted.
6653-
*/
6654-
if (unlikely(flags & __GFP_THISNODE) && nid != NUMA_NO_NODE &&
6655-
nid != page_to_nid(virt_to_page(p)))
6656-
goto alloc_new;
6657-
66586648
if (is_kfence_address(p)) {
66596649
ks = orig_size = kfence_ksize(p);
66606650
} else {
@@ -6673,6 +6663,16 @@ __do_krealloc(const void *p, size_t new_size, unsigned long align, gfp_t flags,
66736663
}
66746664
}
66756665

6666+
/*
6667+
* If reallocation is not necessary (e. g. the new size is less
6668+
* than the current allocated size), the current allocation will be
6669+
* preserved unless __GFP_THISNODE is set. In the latter case a new
6670+
* allocation on the requested node will be attempted.
6671+
*/
6672+
if (unlikely(flags & __GFP_THISNODE) && nid != NUMA_NO_NODE &&
6673+
nid != page_to_nid(virt_to_page(p)))
6674+
goto alloc_new;
6675+
66766676
/* If the old object doesn't fit, allocate a bigger one */
66776677
if (new_size > ks)
66786678
goto alloc_new;
@@ -6707,7 +6707,7 @@ __do_krealloc(const void *p, size_t new_size, unsigned long align, gfp_t flags,
67076707
if (ret && p) {
67086708
/* Disable KASAN checks as the object's redzone is accessed. */
67096709
kasan_disable_current();
6710-
memcpy(ret, kasan_reset_tag(p), orig_size ?: ks);
6710+
memcpy(ret, kasan_reset_tag(p), min(new_size, (size_t)(orig_size ?: ks)));
67116711
kasan_enable_current();
67126712
}
67136713

@@ -6941,7 +6941,7 @@ void *kvrealloc_node_align_noprof(const void *p, size_t size, unsigned long alig
69416941
if (p) {
69426942
/* We already know that `p` is not a vmalloc address. */
69436943
kasan_disable_current();
6944-
memcpy(n, kasan_reset_tag(p), ksize(p));
6944+
memcpy(n, kasan_reset_tag(p), min(size, ksize(p)));
69456945
kasan_enable_current();
69466946

69476947
kfree(p);

0 commit comments

Comments
 (0)