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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250123222002.8897374343971b0b8e877307@linux-foundation.org>
Date: Thu, 23 Jan 2025 22:20:02 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Liu Shixin <liushixin2@...wei.com>
Cc: Kefeng Wang <wangkefeng.wang@...wei.com>, Kemeng Shi
 <shikemeng@...weicloud.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Mel Gorman <mgorman@...hsingularity.net>, David Hildenbrand
 <david@...hat.com>, Matthew Wilcox <willy@...radead.org>, Nanyong Sun
 <sunnanyong@...wei.com>, <linux-mm@...ck.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm/compaction: fix UBSAN shift-out-of-bounds warning

On Thu, 23 Jan 2025 10:10:29 +0800 Liu Shixin <liushixin2@...wei.com> 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.