Re: [PATCH v2 1/2] mm/page_isolation: fix UBSAN shift-out-of-bounds warning

"Zi Yan" <[email protected]>
Newsgroups org.kvack.linux-mm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Thu Aug 20, 2026 at 10:55 PM EDT, Qi Xi wrote:
> A contig-range allocation racing with buddy allocation on the adjacent
> pageblock can trigger:
>
>  UBSAN: shift-out-of-bounds in mm/page_isolation.c:393:15
>  shift exponent -749042176 is negative
>  Call trace:
>   isolate_single_pageblock
>   start_isolate_page_range
>   alloc_contig_frozen_range_noprof
>   alloc_contig_range_noprof
>
> isolate_single_pageblock() first calls set_migratetype_isolate() with
> zone->lock held, which marks the pageblock MIGRATE_ISOLATE and moves any
> free page straddling the boundary out of the way.  Once the lock is
> dropped, it scans the MAX_ORDER_NR_PAGES-aligned window [start_pfn,
> boundary_pfn) locklessly, only to skip the free pages already handled
> above and to detect in-use pages straddling the boundary.  Since this
> scan only reads page state to decide how far to skip and returns -EBUSY
> on a straddling in-use page, it does not take the lock.
>
> The window also covers the adjacent pageblock, whose free pages stay on
> the normal movable/CMA freelist and can be allocated concurrently.  So
> after the scan observes PageBuddy(page), another CPU can allocate the
> page, leaving a stale value in page->private that makes "1 << order" shift
> out of range.
>
> Use buddy_order_unsafe() with READ_ONCE to read the order, and validate
> it is within MAX_PAGE_ORDER before shifting to prevent UBSAN warnings.
>
> Since pageblock_isolate_and_move_free_pages() already handles free pages
> straddling boundary_pfn under zone->lock, bail out with -EBUSY instead
> of VM_WARN_ON_ONCE() when a PageBuddy page appears to cross the boundary
> during the lockless scan.
>
> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
> Cc: [email protected]
> Signed-off-by: Qi Xi <[email protected]>
> ---
>  mm/page_isolation.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 32ce8a7d9df3..f2b648a68531 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -387,13 +387,15 @@ static int isolate_single_pageblock(unsigned long boundary_pfn,
>  		}
>  
>  		if (PageBuddy(page)) {
> -			int order = buddy_order(page);
> +			unsigned int order = buddy_order_unsafe(page);
>  
> -			/* pageblock_isolate_and_move_free_pages() handled this */
> -			VM_WARN_ON_ONCE(pfn + (1 << order) > boundary_pfn);
> -
> -			pfn += 1UL << order;
> -			continue;
> +			/* buddy_order_unsafe() is racy. Validate the order before shifting. */
> +			if (order <= MAX_PAGE_ORDER &&

Can you add a comment like below to clarify the check? Thanks.
				/*
				 * pageblock_isolate_and_move_free_pages() splits
				 * cross-boundary PageBuddy, verify it.
				 */
> +			    pfn + (1UL << order) <= boundary_pfn) {
> +				pfn += 1UL << order;
> +				continue;
> +			}
> +			goto failed;
>  		}
>  
>  		/*


With that, feel free to add

Reviewed-by: Zi Yan <[email protected]>


-- 
Best Regards,
Yan, Zi
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.