Skip to contentSkip to conversation

mm/vmalloc: fix 32-bit truncation in the vrealloc() grow-in-place check

PATCH Superseded Artem Lytkin 1 participant 1 message Jul 25

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 mm/vmalloc: fix 32-bit truncation in the vrealloc() grow-in-place check

Commit d57ac904ffdc ("mm/vmalloc: use physical page count for vrealloc() grow-in-place check") introduced

	if (size <= vm->nr_pages << PAGE_SHIFT) {

in place of a comparison against a size_t. As vm->nr_pages is an unsigned int and a shift is evaluated in the type of its promoted left operand, the right hand side is computed in 32-bit arithmetic and wraps for areas of 4 GiB or more. Twelve lines above, in the same function, four expressions cast for exactly this reason:

	kmemleak_free_part((void *)addr + ((unsigned long)new_nr_pages
					   << PAGE_SHIFT), ...);

The consequence here is benign. Truncation only ever makes the right hand side smaller than the real capacity, so the test can only fail when it should have succeeded: vrealloc() then falls through to need_realloc, allocates a new area, copies min(size, old_size) bytes and frees the old one. That is correct, only wasteful, and no in-tree caller currently grows an allocation past 4 GiB, so this is a latent fix rather than a user-visible one.

Widen the shift to match the surrounding code. Note that comparing page counts instead, as in PAGE_ALIGN(size) >> PAGE_SHIFT <= vm->nr_pages, would avoid the cast but introduce a real bug: PAGE_ALIGN() wraps to 0 for sizes within a page of ULONG_MAX and the test would then wrongly succeed.

Fixes d57ac904ffdc ("mm/vmalloc: use physical page count for vrealloc() grow-in-place check")
Assisted-by Claude:claude-fable-5
---
 mm/vmalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/vmalloc.c +1 -1
index 44647e189f7d6..baf7e396e3fc7 100644
@@ -4415,7 +4415,7 @@ void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align
 	/*
 	 * We already have the bytes available in the allocation; use them.
 	 */
-	if (size <= vm->nr_pages << PAGE_SHIFT) {
+	if (size <= (unsigned long)vm->nr_pages << PAGE_SHIFT) {
 		/*
 		 * No need to zero memory here, as unused memory will have
 		 * already been zeroed at initial allocation time or during
Signature · 1 line
2.43.0