From: Uladzislau Rezki <urezki@gmail.com>
To: Artem Lytkin <iprintercanon@gmail.com>
Cc: linux-mm@kvack.org, akpm@linux-foundation.org, urezki@gmail.com,
shivamkalra98@zohomail.in, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/vmalloc: make vm_struct.nr_pages an unsigned long
Date: Thu, 30 Jul 2026 15:04:24 +0200 [thread overview]
Message-ID: <amtL2Md-vl_gt-pV@milan> (raw)
In-Reply-To: <20260729175708.7074-1-iprintercanon@gmail.com>
On Wed, Jul 29, 2026 at 08:57:08PM +0300, Artem Lytkin wrote:
> A vmalloc area can hold more than 2^32 pages, but the page counts
> tracking it are 32 bit. That has produced the same truncation bug twice
> already, once in vread_iter() and once in the vrealloc() grow-in-place
> check, and it keeps the casts that hide it scattered around the file.
>
> Widen the field and everything that feeds or consumes it, so the byte
> counts derived from it are computed in 64 bit arithmetic without a cast
> at each site:
>
> - vm_area_alloc_pages() takes and returns a page count, and the result
> is stored into nr_pages, so its parameter, return type and the
> nr_allocated and nr_remaining accumulators widen too. The 100 page
> cap on a bulk request stays narrow, only its literal becomes 100UL.
>
> - nr_small_pages in __vmalloc_area_node() is derived from a 64 bit
> size and compared against nr_pages, so it widens as well. Before
> this the store truncated for a size of 16 TiB or more, which sized
> area->pages from the wrapped count while __vmap_pages_range() still
> walked the full size. The cast on array_size next to it was already
> redundant and goes away with it.
>
> - new_nr_pages and old_nr_pages in vrealloc_node_align_noprof() widen,
> which drops the four casts on them, and the casts added by the two
> preceding patches in vrealloc_node_align_noprof() and vread_iter()
> are no longer needed either.
>
> - vm_area_free_pages() takes a page index range.
>
> Three loop counters that index area->pages were plain int and would have
> overflowed at 2^31 pages: in set_area_direct_map(), in vm_reset_perms()
> and in the bulk allocation loop, where the index is seeded from
> nr_allocated. They become unsigned long.
>
> Printing needed fixing in two places. vmalloc_dump_obj() used %u, and
> vmalloc_info_show() printed the unsigned field with %d, which would have
> shown a negative page count for an area of 8 TiB or more.
> show_numa_info() used a single unsigned int as both the page index and
> the node id, so the page loop gets its own unsigned long counter.
>
> The page table walkers are not covered here. vmap_pages_pte_range() and
> the levels above it carry the page cursor as int *nr, so a mapping is
> still capped independently of the counters this patch widens. Widening
> that cursor touches the whole walker and is separate work.
>
> Nothing outside mm/vmalloc.c needs a change to build or to behave as
> before. Those users either index the pages array or multiply the count
> by PAGE_SIZE, which is unsigned long already. kexec_handover stores the
> count into a 32 bit field of its own ABI, which caps that interface
> exactly where it did before.
>
> On x86-64 this leaves sizeof(struct vm_struct) at 72 bytes when
> CONFIG_HAVE_ARCH_HUGE_VMALLOC=n, since the existing padding before
> phys_addr absorbs the change, and takes it to 80 when the config is
> enabled. Both sizes come from the same kmalloc-96 bucket that
> __get_vm_area_node() allocates from, so the footprint per area does not
> change either way.
>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
> include/linux/vmalloc.h | 2 +-
> mm/vmalloc.c | 62 ++++++++++++++++++++---------------------
> 2 files changed, 31 insertions(+), 33 deletions(-)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index d87dc7f77f4e8..a39c20c77efcd 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -62,7 +62,7 @@ struct vm_struct {
> #ifdef CONFIG_HAVE_ARCH_HUGE_VMALLOC
> unsigned int page_order;
> #endif
> - unsigned int nr_pages;
> + unsigned long nr_pages;
> phys_addr_t phys_addr;
> const void *caller;
> unsigned long requested_size;
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index baf7e396e3fc7..efd5e42c64e2e 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3338,7 +3338,7 @@ struct vm_struct *remove_vm_area(const void *addr)
> static inline void set_area_direct_map(const struct vm_struct *area,
> int (*set_direct_map)(struct page *page))
> {
> - int i;
> + unsigned long i;
>
> /* HUGE_VMALLOC passes small pages to set_direct_map */
> for (i = 0; i < area->nr_pages; i++)
> @@ -3354,7 +3354,7 @@ static void vm_reset_perms(struct vm_struct *area)
> unsigned long start = ULONG_MAX, end = 0;
> unsigned int page_order = vm_area_page_order(area);
> int flush_dmap = 0;
> - int i;
> + unsigned long i;
>
> /*
> * Find the start and end range of the direct mappings to make sure that
> @@ -3427,10 +3427,10 @@ void vfree_atomic(const void *addr)
> * Caller is responsible for unmapping (vunmap_range) and KASAN
> * poisoning before calling this.
> */
> -static void vm_area_free_pages(struct vm_struct *vm, unsigned int start_idx,
> - unsigned int end_idx)
> +static void vm_area_free_pages(struct vm_struct *vm, unsigned long start_idx,
> + unsigned long end_idx)
> {
> - unsigned int i;
> + unsigned long i;
>
> if (!(vm->flags & VM_MAP_PUT_PAGES)) {
> for (i = start_idx; i < end_idx; i++)
> @@ -3642,12 +3642,12 @@ static inline gfp_t vmalloc_gfp_adjust(gfp_t flags, const bool large)
> return flags;
> }
>
> -static inline unsigned int
> +static inline unsigned long
> vm_area_alloc_pages(gfp_t gfp, int nid,
> - unsigned int order, unsigned int nr_pages, struct page **pages)
> + unsigned int order, unsigned long nr_pages, struct page **pages)
> {
> - unsigned int nr_allocated = 0;
> - unsigned int nr_remaining = nr_pages;
> + unsigned long nr_allocated = 0;
> + unsigned long nr_remaining = nr_pages;
> unsigned int max_attempt_order = MAX_PAGE_ORDER;
> struct page *page;
> int i;
> @@ -3695,7 +3695,7 @@ vm_area_alloc_pages(gfp_t gfp, int nid,
> if (!order) {
> while (nr_allocated < nr_pages) {
> unsigned int nr, nr_pages_request;
> - int i;
> + unsigned long i;
>
> /*
> * A maximum allowed request is hard-coded and is 100
> @@ -3703,7 +3703,7 @@ vm_area_alloc_pages(gfp_t gfp, int nid,
> * long preemption off scenario in the bulk-allocator
> * so the range is [1:100].
> */
> - nr_pages_request = min(100U, nr_pages - nr_allocated);
> + nr_pages_request = min(100UL, nr_pages - nr_allocated);
>
> /* memory allocation should consider mempolicy, we can't
> * wrongly use nearest node when nid == NUMA_NO_NODE,
> @@ -3849,12 +3849,12 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> unsigned long addr = (unsigned long)area->addr;
> unsigned long size = get_vm_area_size(area);
> unsigned long array_size;
> - unsigned int nr_small_pages = size >> PAGE_SHIFT;
> + unsigned long nr_small_pages = size >> PAGE_SHIFT;
> unsigned int page_order;
> unsigned int flags;
> int ret;
>
> - array_size = (unsigned long)nr_small_pages * sizeof(struct page *);
> + array_size = nr_small_pages * sizeof(struct page *);
>
> /* __GFP_NOFAIL and "noblock" flags are mutually exclusive. */
> if (!gfpflags_allow_blocking(gfp_mask))
> @@ -4352,7 +4352,7 @@ void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align
> }
>
> if (size <= old_size) {
> - unsigned int new_nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> + unsigned long new_nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>
> /* Zero out "freed" memory, potentially for future realloc. */
> if (want_init_on_free() || want_init_on_alloc(flags))
> @@ -4381,7 +4381,7 @@ void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align
> !(vm->flags & (VM_FLUSH_RESET_PERMS | VM_USERMAP)) &&
> gfp_has_io_fs(flags)) {
> unsigned long addr = (unsigned long)kasan_reset_tag(p);
> - unsigned int old_nr_pages = vm->nr_pages;
> + unsigned long old_nr_pages = vm->nr_pages;
>
> /*
> * Use the node lock to synchronize with concurrent
> @@ -4394,16 +4394,13 @@ void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align
> spin_unlock(&vn->busy.lock);
>
> /* Notify kmemleak of the reduced allocation size before unmapping. */
> - kmemleak_free_part(
> - (void *)addr + ((unsigned long)new_nr_pages
> - << PAGE_SHIFT),
> - (unsigned long)(old_nr_pages - new_nr_pages)
> - << PAGE_SHIFT);
> + kmemleak_free_part((void *)addr +
> + (new_nr_pages << PAGE_SHIFT),
> + (old_nr_pages - new_nr_pages)
> + << PAGE_SHIFT);
>
> - vunmap_range(addr + ((unsigned long)new_nr_pages
> - << PAGE_SHIFT),
> - addr + ((unsigned long)old_nr_pages
> - << PAGE_SHIFT));
> + vunmap_range(addr + (new_nr_pages << PAGE_SHIFT),
> + addr + (old_nr_pages << PAGE_SHIFT));
>
> vm_area_free_pages(vm, new_nr_pages, old_nr_pages);
> }
> @@ -4415,7 +4412,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 <= (unsigned long)vm->nr_pages << PAGE_SHIFT) {
> + 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
> @@ -4722,7 +4719,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) ?
> - ((unsigned long)vm->nr_pages << PAGE_SHIFT) :
> + (vm->nr_pages << PAGE_SHIFT) :
> get_vm_area_size(vm);
> else
> size = va_size(va);
> @@ -5228,7 +5225,7 @@ bool vmalloc_dump_obj(void *object)
> struct vmap_area *va;
> struct vmap_node *vn;
> unsigned long addr;
> - unsigned int nr_pages;
> + unsigned long nr_pages;
>
> addr = PAGE_ALIGN((unsigned long) object);
> vn = addr_to_node(addr);
> @@ -5248,7 +5245,7 @@ bool vmalloc_dump_obj(void *object)
> nr_pages = vm->nr_pages;
> spin_unlock(&vn->busy.lock);
>
> - pr_cont(" %u-page vmalloc region starting at %#lx allocated at %pS\n",
> + pr_cont(" %lu-page vmalloc region starting at %#lx allocated at %pS\n",
> nr_pages, addr, caller);
>
> return true;
> @@ -5266,16 +5263,17 @@ bool vmalloc_dump_obj(void *object)
> static void show_numa_info(struct seq_file *m, struct vm_struct *v,
> unsigned int *counters)
> {
> - unsigned int nr;
> unsigned int step = 1U << vm_area_page_order(v);
> + unsigned long i;
> + unsigned int nr;
>
> if (!counters)
> return;
>
> memset(counters, 0, nr_node_ids * sizeof(unsigned int));
>
> - for (nr = 0; nr < v->nr_pages; nr += step)
> - counters[page_to_nid(v->pages[nr])] += step;
> + for (i = 0; i < v->nr_pages; i += step)
> + counters[page_to_nid(v->pages[i])] += step;
> for_each_node_state(nr, N_HIGH_MEMORY)
> if (counters[nr])
> seq_printf(m, " N%u=%u", nr, counters[nr]);
> @@ -5333,7 +5331,7 @@ static int vmalloc_info_show(struct seq_file *m, void *p)
> seq_printf(m, " %pS", v->caller);
>
> if (v->nr_pages)
> - seq_printf(m, " pages=%d", v->nr_pages);
> + seq_printf(m, " pages=%lu", v->nr_pages);
>
> if (v->phys_addr)
> seq_printf(m, " phys=%pa", &v->phys_addr);
>
> base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
> prerequisite-patch-id: bc462c518eb9ddcaccad5e1f5fed7cc24512b117
> prerequisite-patch-id: 891688c2e93c570d1c9c7802b46aaba974002684
> --
> 2.43.0
>
Makes sense to me. The commit message could be shorter describing
just an issue. I was lost when i read it first time.
Also:
<snip>
Artem Lytkin (3):
mm/vmalloc: fix 32-bit truncation of the area size in vread_iter()
mm/vmalloc: fix 32-bit truncation in the vrealloc() grow-in-place check
mm/vmalloc: make vm_struct.nr_pages an unsigned long
<snip>
first two fix overlaps. The latest also do it by converting nr_pages
to unsigned long and removes (unsigned long) casting introduced by 1 and 2.
Maybe just use one commit which is last in this series?
Anyway for this
Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
--
Uladzislau Rezki
prev parent reply other threads:[~2026-07-30 13:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-29 17:57 [PATCH] mm/vmalloc: make vm_struct.nr_pages an unsigned long Artem Lytkin
2026-07-29 18:28 ` Matthew Wilcox
2026-07-29 21:45 ` Andrew Morton
2026-07-30 9:07 ` Artem Lytkin
2026-07-30 9:06 ` [PATCH v2 0/3] mm/vmalloc: stop truncating byte counts derived from nr_pages Artem Lytkin
2026-07-30 9:06 ` [PATCH v2 1/3] mm/vmalloc: fix 32-bit truncation of the area size in vread_iter() Artem Lytkin
2026-07-30 9:06 ` [PATCH v2 2/3] mm/vmalloc: fix 32-bit truncation in the vrealloc() grow-in-place check Artem Lytkin
2026-07-30 9:06 ` [PATCH v2 3/3] mm/vmalloc: make vm_struct.nr_pages an unsigned long Artem Lytkin
2026-07-30 13:04 ` Uladzislau Rezki [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=amtL2Md-vl_gt-pV@milan \
--to=urezki@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=iprintercanon@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=shivamkalra98@zohomail.in \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox