Re: [PATCH] mm/huge_memory: Fix iterator variable usage after swap()

[David Hildenbrand (Red Hat)]

On 1/21/26 10:25, Lance Yang wrote:
> On 2026/1/21 16:13, zenghongling wrote:
> > The iterator variable 'folio' is swapped with 'prev' in the else
> > branch. Using 'folio' after swap() checks the potentially NULL
> > 'prev' value, not the original iterator value.
> >
> > Fix by moving folio_put() call before the swap operation in the
> > path where swap() occurs.
> >
> > Found by:
> > ./huge_memory.c:4225:6-11: ERROR: iterator variable bound on line 4178 cannot be NULL

Which tool did find that? A compiler?

> Good catch!
>
> But which tree is your patch based on?
>
> Seems like that was already fixed in commit 776bde7caf80[1]. The
> whole thing deferred_split_scan() was refactored using folio_batch,
> so the buggy code with swap(folio, prev) is gone ...
>
> Ccing Muchun and Qi who fixed that.
>
> [1]
> https://lore.kernel.org/all/59cb6b6fb5ffcff9d23b81890b252960139ad8e7.1762762324.git.zhengqi.arch@xxxxxxxxxxxxx/

Right, in

commit 776bde7caf80f6af72b087cafe7d9f607b14716d
Author: Muchun Song <muchun.song@xxxxxxxxx>
Date:   Mon Nov 10 16:17:57 2025 +0800

    mm: thp: use folio_batch to handle THP splitting in deferred_split_scan()


Which raises the question whether we would want to backport that patch to stable kernels
if there was indeed a problem?


But: I don't immediately see the problem.

If pref is NULL (and folio obviously !+NULL), we'll end up with
	* pref != NULL
	* folio == NULL

	The "if (folio)" check will do nothing, because we defer the freeing to the

		if (prev)
			folio_put(prev);

	later

If pref is != NULL (and folio obviously !+NULL), we'll end up with
	* pref = NULL
	* folio = NULL

	The if (folio) and if (prev) handling will care of it all.


So ... this pretty much looks like working as expected?

--
Cheers

David