Re: [PATCH] mm/compaction: fix UBSAN shift-out-of-bounds warning

[Andrew Morton]

On Thu, 23 Jan 2025 10:10:29 +0800 Liu Shixin <liushixin2@xxxxxxxxxx> wrote:

> syzkaller reported a UBSAN shift-out-of-bounds warning of (1UL << order)

A Link: to the syzcaller report would be great, please.

> in isolate_freepages_block(). The bogus compound_order can be any value
> because it is union with flags. Add back the MAX_PAGE_ORDER check to fix
> the warning.

OK, I'd never noticed compound_order()'s restrictions before.  It looks
like a crazy thing - what use is it if it can return "wild return
values"?

Can someone please explain what's going on here and suggest what we can
do about it?

For example, should we have a compound_order_not_wild() which is called
with refcounted pages and which cannot return "wild" numbers?  Or
something else.

> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -630,7 +630,8 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
>  		if (PageCompound(page)) {
>  			const unsigned int order = compound_order(page);
>  
> -			if (blockpfn + (1UL << order) <= end_pfn) {
> +			if ((order <= MAX_PAGE_ORDER) &&
> +			    (blockpfn + (1UL << order) <= end_pfn)) {
>  				blockpfn += (1UL << order) - 1;
>  				page += (1UL << order) - 1;
>  				nr_scanned += (1UL << order) - 1;

isolate_migratepages_block()'s

		if (skip_isolation_on_order(order, cc->order)) {

doesn't check for "wild" values, but it seems that
skip_isolation_on_order() will handle it.