mm/vmalloc: fix 32-bit truncation in the vrealloc() grow-in-place check
Subsystems (2)
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.
--- mm/vmalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/vmalloc.c +1 -1
@@ -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