On Tue, Nov 10, 2026 at 12:26 PM Hao Ge <[email protected]> wrote: > > > > On 2026/11/10 19:25, Hao Ge wrote: > > commit a739e6b557af ("mm: vmalloc: WARN_ON if mapping size is not > > PAGE_SIZE aligned") put the check in vmap_pte_range(), which is too > > late, because the upper levels may already have installed page > > tables for a range that is rejected anyway. > > > > The pte loop maps whole pages and stops only when addr lands > > exactly on end. An unaligned end lets addr jump over it and the > > loop keeps mapping past the range. An unaligned start maps the > > page below the range and usually misses end too. Such a range is > > never a legitimate request, and the corruption it leaves behind is > > hard to attribute to the caller afterwards, so bug out on addr and > > end at the entry of vmap_range_noflush() and drop the check from > > vmap_pte_range(). > > > > Signed-off-by: Hao Ge <[email protected]> > > --- > > mm/vmalloc.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > > index 093b8d5f7b0e..27f2674a5556 100644 > > --- a/mm/vmalloc.c > > +++ b/mm/vmalloc.c > > @@ -125,9 +125,6 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long > > addr, unsigned long end, > > unsigned long size; > > unsigned int nr_pages; > > > > - if (WARN_ON_ONCE(!PAGE_ALIGNED(end - addr))) > > - return -EINVAL; > > - > > pfn = phys_addr >> PAGE_SHIFT; > > pte = pte_alloc_kernel_track(pmd, addr, mask); > > if (!pte) > > @@ -347,6 +344,7 @@ static int vmap_range_noflush(unsigned long addr, > > unsigned long end, > > */ > > might_sleep(); > > BUG_ON(addr >= end); > > + BUG_ON(!PAGE_ALIGNED(addr) || !PAGE_ALIGNED(end)); > > I know BUG_ON() is ugly and it was not my first choice either, but I > have not figured out a better way to handle __vunmap_range_noflush(). > > If an unaligned range is passed to it, the pte loop may clear PTEs > adjacent to the range. Say a caller passes [3, 12) while [0, 3) > belongs to somebody else. The loop operates on whole pages, so it > starts at the page containing 3 and clears [0, 3) with it. That > silently corrupts a live mapping, and the corruption is not detected > until the owner touches the page. Bugs like that are nearly > impossible to debug afterwards. > > It is not feasible for __vunmap_range_noflush() to just return an > invalid-argument error upfront. Its job is to clear PTEs, and an > early return leaves the residual PTEs behind. > > I also thought about warning and returning -EINVAL on the map side > and keeping BUG_ON() only in __vunmap_range_noflush(), but the > rollback kills that idea. > > int __vmap_pages_range_noflush(unsigned long addr, unsigned long end, > pgprot_t prot, struct page **pages, unsigned int page_shift, > gfp_t gfp_mask) > { > int err; > > if (WARN_ON_ONCE(page_shift < PAGE_SHIFT)) > return -EINVAL; > > if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC)) > page_shift = PAGE_SHIFT; > > err = vmap_pages_range_noflush_walk(addr, end, prot, pages, > page_shift); > > /* Undo the PTEs installed before the failure. */ > if (err) > __vunmap_range_noflush(addr, end, > gfpflags_allow_blocking(gfp_mask)); > > return err; > } > > If the walk WARNs and returns -EINVAL for a bad range, the undo > above passes that same range to __vunmap_range_noflush(), and we > BUG right after the WARN. vmap_page_range() runs into the same > thing on its error path. > > So the map side cannot warn while the unmap side BUGs, the rollback > drags the rejected range into the unmap anyway. And the unmap side > cannot warn and skip either, that leaves the stale PTEs. > > Also, these checks are the last line of defense. All in-tree callers > pass page aligned, non-empty ranges, and I am not aware of anyone ever > hitting this line. So for correct code the BUG_ON never fires at all, > no user will ever see it. The only one who can trip it is the developer > who just passed the bad range, and for him a stack trace right at the > spot is much better than chasing a silent corruption afterwards. > > I settled on BUG_ON here. It may not be an ideal solution, but if anyone > has a better idea or alternative approach, please let me know. > I would greatly appreciate it.
I don't see a better way to handle this situation but I would add a WARN_ON_ONCE() alignment check at the beginning of vmap_page_range() too because it is used in some of the exported functions like ioremap_page() and pci_remap_iospace(). If someone passes an unaliged address to these functions your change would turn a warning+function failure into a panic. > > Thanks > Best Regards > Hao > > > > > start = addr; > > pgd = pgd_offset_k(addr);
