On 2025/1/24 14:20, Andrew Morton wrote:
> On Thu, 23 Jan 2025 10:10:29 +0800 Liu Shixin <liushixin2@huawei.com> wrote:
>
>> syzkaller reported a UBSAN shift-out-of-bounds warning of (1UL << order)
> A Link: to the syzcaller report would be great, please.
The warning arises only once in another version instead of mainline, and the syzkaller log
not reproduce. In that version, compound_orderis still union with lru instead of flags,
so I didn't put it in the log.
See enum pageflags, we can know that the warning need folio setting PG_waiters flags,
which is low probability.
>
>> 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.
> .
>