mm/vmalloc: fix 32-bit truncation of the area size in vread_iter()

Commit 0bca23804632 ("mm/vmalloc: use physical page count in vread_iter() for VM_ALLOC areas") replaced get_vm_area_size(vm), which returns a size_t, with vm->nr_pages << PAGE_SHIFT.

struct vm_struct::nr_pages is an unsigned int. The shift operator does not perform the usual arithmetic conversions: the integer promotions are applied to each operand and the type of the result is that of the promoted left operand. The expression is therefore evaluated in 32-bit arithmetic no matter how PAGE_SHIFT is typed, and no matter that the result is assigned to a size_t. Once an area reaches 4 GiB the byte count wraps, at 1 << 20 pages with 4 KiB pages, 1 << 18 with 16 KiB and 1 << 16 with 64 KiB.

nr_pages counts base pages even for huge vmalloc mappings, since __vmalloc_area_node() requires area->nr_pages == nr_small_pages, so a huge page_order does not raise the threshold.

The only limit on the size of a single area is the totalram_pages() check in __vmalloc_node_range_noprof(), so any machine with more than 4 GiB of memory can create an affected area. One example is the zram metadata table, which is a single vzalloc() of disksize / 256 with CONFIG_LOCKDEP=n: "echo 1T > /sys/block/zram0/disksize" allocates exactly 4 GiB and needs no 1 TiB of anything. Sufficiently large BPF array or prealloc-hash maps do it too, as does
"modprobe test_vmalloc run_test_mask=1 nr_pages=1048576". Two paths that might be expected to reach it cannot: alloc_large_system_hash() caps a table at a sixteenth of memory, and the KVM dirty bitmap is bounded by KVM_MEM_MAX_NR_PAGES to 512 MiB.

The effect is confined to /proc/kcore, the only caller. For an area whose size is an exact multiple of 4 GiB the computed size becomes 0, the

	if (addr >= vaddr + size)
		goto next_va;

test succeeds and the whole area is skipped; for other sizes only the first nr_pages mod 2^20 pages are read and the rest is skipped. Either way the bytes are zero-filled at the finished_zero label, which returns the full requested length, so read_kcore_iter() sees a successful read and userspace gets neither an error nor a short read. Live inspection with drgn, crash or gdb silently observes zeros where the area is populated, and cannot distinguish that from genuinely zeroed memory. Before the offending commit the size came from vm->size in 64-bit arithmetic, so this is a v7.2 regression.

The truncated value is always less than or equal to the true size, so vread_iter() can only under-read; there is no out-of-bounds access. Both the affected reader and every producer above are privileged: opening /proc/kcore requires CAP_SYS_RAWIO and is refused under lockdown. This is a correctness and debuggability problem, not a security one.

Fix it by widening the shift, which also makes the expression consistent with the four (unsigned long)nr_pages << PAGE_SHIFT expressions in vrealloc_node_align_noprof().

On 32-bit a widening cast cannot help, size_t being 32 bits there as well, but a 4 GiB vmalloc area is not reachable on 32-bit either. On 64-bit the cast removes the truncation entirely, which is why replacing get_vm_area_size() introduced a regression rather than inheriting a pre-existing wart.

---
 mm/vmalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 1afca3568b9b6..44647e189f7d6 100644
@@ -4722,7 +4722,7 @@ long vread_iter(struct iov_iter *iter, const char *addr, size_t count)
 			 * mapping types (vmap, ioremap) don't set nr_pages.
 			 */
 			size = (vm->flags & VM_ALLOC && vm->nr_pages) ?
-				       (vm->nr_pages << PAGE_SHIFT) :
+				       ((unsigned long)vm->nr_pages << PAGE_SHIFT) :
 				       get_vm_area_size(vm);
 		else
 			size = va_size(va);

base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df

2.43.0

Thanks. AI review might have found a few things. Most are pre-existing but they are basically "more of the same thing", so you may choose to address them?

	https://sashiko.dev/#/patchset/[email protected]

I wonder how much of this stuff would go away if we were to make vm_struct.nr_pages an unsigned long? It's already using 64 bits in the CONFIG_HAVE_ARCH_HUGE_VMALLOC=n case.

All the casts, and it's free.

sizeof(struct vm_struct) is 72 today either way. As unsigned long it stays 72 with HUGE_VMALLOC=n, the 4 byte hole before phys_addr takes it, and goes to 80 with =y where page_order and nr_pages share a slot. Both land in kmalloc-96, which is what __get_vm_area_node() allocates from, so nothing really grows.

That kills both casts here plus the four in vrealloc_node_align_noprof(), as long as new_nr_pages and old_nr_pages get widened with it. Nothing outside mm/vmalloc.c needs touching.

It doesn't get all the narrowing though: vm_area_alloc_pages() still takes and returns unsigned int, nr_small_pages is its own local off size, "pages=%d" wants %lu, and show_numa_info() uses one unsigned int for both the page index and the node id.

I'd rather not fold that in here, 1/2 is the kcore regression and the bit worth backporting. I'll send the widening on top with all of the above in it.

As for the findings: the vrealloc truncation is 4418, which is 2/2 here, and there's nothing else narrow left in that function.

nr_small_pages is real but needs more than 16 TiB of RAM, since __vmalloc_node_range_noprof() checks size >> PAGE_SHIFT against totalram_pages() first, and I couldn't find a caller allocating that much in one go. Fixing it alone buys nothing while the other counts are 32 bit, so it goes in the widening patch.

The __GFP_ZERO one I don't think is a bug. The shrink path zeroes when want_init_on_free() or want_init_on_alloc() is set, and the kerneldoc already requires callers passing __GFP_ZERO to pass it on every call.

Thanks,
Artem

I was thinking about it in same direction. Just to convert and get rid of casting. It is safe.

--
Uladzislau Rezki