Re: [PATCH v2] mm: page_isolation: avoid unsafe folio reads while scanning compound pages

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/2/26 17:02, Zi Yan wrote:
> On 2 Jun 2026, at 9:07, Kaitao Cheng wrote:
> 
>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>
>> page_is_unmovable() can inspect compound pages without holding a folio
>> reference or any lock. The folio can therefore be freed, split or reused
>> while the scanner is still looking at it.
>>
>> The existing HugeTLB handling already avoids folio_hstate() for this
>> reason, but it still derives the hstate from folio_size() and later
>> derives the scan step from folio_nr_pages() and folio_page_idx().
>> These helpers rely on the folio still being a valid folio head. If
>> the folio changed concurrently, the scanner can read inconsistent folio
>> metadata and compute a wrong step. In the worst case, folio_nr_pages()
>> can return 1 for what used to be a tail page and the subtraction from
>> folio_page_idx() can underflow.
>>
>> There is a similar issue for non-Hugetlb compound pages: folio_test_lru()
>> expects a valid folio. If the previously observed head page has been
>> reused as a tail page of another compound page, the folio flag checks
>> can trigger VM_BUG_ON_PGFLAGS().
>>
>> Read the compound order once with compound_order(), reject obviously
>> bogus orders, and derive the hstate and scan step from that order
>> instead of querying folio size information again. Also use PageLRU(page),
>> which is safe for the page being scanned, instead of folio_test_lru()
>> on a potentially stale folio pointer.
>>
>> Treat an unknown HugeTLB hstate as unmovable so the scanner does not try
>> to skip over an unstable HugeTLB folio.
>>
>> Fixes: a0a9f2180b90 ("mm: page_isolation: avoid calling folio_hstate() without hugetlb_lock")
>> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
>> ---
>> Changes in v2:
>> - Avoid unsafe folio metadata reads in the unlocked scanner by deriving
>>   the hstate and scan step from compound_order(). (David Hildenbrand,
>>   Andrew Morton)
>> - Treat invalid compound orders or unknown HugeTLB hstates as unmovable.
>> - Use PageLRU(page) instead of folio_test_lru(folio) to avoid folio flag
>>   checks on a stale folio pointer. ()
>> - Update the commit log (David Hildenbrand)
>>
>> Link to v1:
>> https://lore.kernel.org/all/20260519121646.40833-1-kaitao.cheng@linux.dev/
>>
>> ---
>>  mm/page_isolation.c | 19 +++++++++++++------
>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> index 7a9d631945a3..32ce8a7d9df3 100644
>> --- a/mm/page_isolation.c
>> +++ b/mm/page_isolation.c
>> @@ -41,8 +41,14 @@ bool page_is_unmovable(struct zone *zone, struct page *page,
>>  	 * We need not scan over tail pages because we don't
>>  	 * handle each tail page individually in migration.
>>  	 */
>> -	if (PageHuge(page) || PageCompound(page)) {
>> +	if (PageCompound(page)) {
>>  		struct folio *folio = page_folio(page);
>> +		unsigned long nr_pages, pfn;
>> +		unsigned int order;
>> +
>> +		order = compound_order(&folio->page);
>> +		if (order > MAX_FOLIO_ORDER)
>> +			return true;
>>
>>  		if (folio_test_hugetlb(folio)) {
>>  			struct hstate *h;
>> @@ -54,15 +60,16 @@ bool page_is_unmovable(struct zone *zone, struct page *page,
>>  			 * The huge page may be freed so can not
>>  			 * use folio_hstate() directly.
>>  			 */
>> -			h = size_to_hstate(folio_size(folio));
>> -			if (h && !hugepage_migration_supported(h))
>> +			h = size_to_hstate(PAGE_SIZE << order);
>> +			if (!h || !hugepage_migration_supported(h))
>>  				return true;
>> -
>> -		} else if (!folio_test_lru(folio)) {
>> +		} else if (!PageLRU(page)) {
>>  			return true;
>>  		}
>>
>> -		*step = folio_nr_pages(folio) - folio_page_idx(folio, page);
>> +		nr_pages = 1UL << order;
>> +		pfn = page_to_pfn(page);
>> +		*step = (pfn | (nr_pages - 1)) + 1 - pfn;
>>  		return false;
>>  	}
> 
> LGTM. Thanks.
> 
> Just a comment, order can be dropped and use
> nr_pages = compound_nr(&folio->page) instead:
> 1. order > MAX_FOLIO_ORDER -> nr_pages > MAX_FOLIO_NR_PAGES
> 2. PAGE_SIZE << order -> PAGE_SIZE * nr_pages.
> But it is not worth a new version.

If we use compound_nr, we have to check for power-of-2, like
scan_movable_pages(),  as folio_large_nr_pages() might use the per-computed
value (which might read garbage).

-- 
Cheers,

David