Re: [PATCH v4] mm/vmalloc: make vm_struct.nr_pages an unsigned long
Uladzislau Rezki <[email protected]> Sun, 2 Aug 2026 17:52:26 +0200
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <am9numIMZZ4s7UMl@milan> |
On Sat, Aug 01, 2026 at 11:52:02AM -0700, Andrew Morton wrote: > On Sat, 1 Aug 2026 14:49:15 +0300 Artem Lytkin <[email protected]> wrote: > > > vm_struct::nr_pages is an unsigned int, and the file keeps deriving byte > > counts from it as nr_pages << PAGE_SHIFT. A shift is evaluated in the type > > of its promoted left operand, so those are 32-bit arithmetic and wrap at > > 4 GiB of bytes, which is 2^20 pages. Every site depends on a cast being > > remembered; vmap() has one, two recent commits did not. vread_iter() then > > computes a size of zero for a 4 GiB VM_ALLOC area and /proc/kcore returns > > it as zeros while reporting a successful read, which drgn, crash or gdb > > cannot tell from real memory, and the vrealloc() grow-in-place check > > declines a request that would have fit. > > > > Widen the field so the class of bug goes away instead of one site at a > > time. Everything feeding or consuming it widens too: vm_area_alloc_pages() > > and its accumulators, nr_small_pages, new_nr_pages and old_nr_pages, the > > index range of vm_area_free_pages(), and three page indexes that were > > plain int. Five casts go. Two prints needed fixing as well, %u in > > vmalloc_dump_obj() and %d for the unsigned field in vmalloc_info_show(). > > > > No bug report behind this, I found it reading the code. The 4 GiB wrap > > needs only a machine with over 4 GiB of memory. Neither larger threshold > > is a practical concern: 2^32 pages, where the field itself truncates, is > > 16 TiB and beyond what hardware can populate, and 2^31, where the plain > > int indexes break, is 8 TiB and larger than anything in the tree asks for. > > The int *nr cursor in the mapping path is unchanged and is separate work. > > Users outside mm/vmalloc.c need no change either. Those handing the count > > to a narrower parameter cannot drive it near 2^31, and > > kho_preserve_vmalloc() stores it into a 32-bit ABI field that still > > receives the same low bits; above 2^32 pages the truncation just moves out > > of vm_struct into that store. > > > > sizeof(struct vm_struct) on x86-64 stays 72 bytes with > > CONFIG_HAVE_ARCH_HUGE_VMALLOC=n and goes from 72 to 80 with it enabled, > > both inside the kmalloc-96 bucket it already comes from. > > Thanks. > > Ulad, AI review suggests that vrealloc() has an issue handling > __GFP_ZERO. Can you please check? > > https://sashiko.dev/#/patchset/[email protected] > I have checked. I think the AI is missing at least one point. AI argument which is: <snip> If a driver initially allocates memory using vmalloc() without __GFP_ZERO (leaving spare page capacity uninitialized), and then grows the allocation using vrealloc() with __GFP_ZERO, the caller expects the newly exposed bytes to be zeroed. <snip> In the vrealloc_node_align_noprof() header documentation there is a statement: <snip> * If __GFP_ZERO logic is requested, callers must ensure that, starting with the * initial memory allocation, every subsequent call to this API for the same * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that * __GFP_ZERO is not fully honored by this API. <snip> AI argument violates the documentation, i.e. mixing __GFP_ZERO is not allowed. From the other hand we can mix it and remove that part of documentation: <snip> diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 7a0cbba3d29d..28d0fed94d22 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -4294,11 +4294,6 @@ EXPORT_SYMBOL(vzalloc_node_noprof); * __GFP_THISNODE flag should be set, otherwise the function will try to avoid * reallocation and possibly disregard the specified @nid. * - * If __GFP_ZERO logic is requested, callers must ensure that, starting with the - * initial memory allocation, every subsequent call to this API for the same - * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that - * __GFP_ZERO is not fully honored by this API. - * * Requesting an alignment that is bigger than the alignment of the existing * allocation will fail. * @@ -4415,13 +4410,12 @@ 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) { - /* - * No need to zero memory here, as unused memory will have - * already been zeroed at initial allocation time or during - * realloc shrink time. - */ - vm->requested_size = size; kasan_vrealloc(p, old_size, size); + + if (want_init_on_alloc(flags)) + memset((void *)p + old_size, 0, size - old_size); + + vm->requested_size = size; return (void *)p; } <snip> -- Uladzislau Rezki