[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f9bfaa03-bc02-4379-b542-457015053e5e@kernel.org>
Date: Wed, 21 Jan 2026 13:28:46 +0100
From: "David Hildenbrand (Red Hat)" <david@...nel.org>
To: Lance Yang <lance.yang@...ux.dev>, zenghongling
<zenghongling@...inos.cn>, Qi Zheng <qi.zheng@...ux.dev>,
Muchun Song <muchun.song@...ux.dev>
Cc: linux-mm@...ck.org, dev.jain@....com, akpm@...ux-foundation.org,
ryan.roberts@....com, baolin.wang@...ux.alibaba.com, npache@...hat.com,
linux-kernel@...r.kernel.org, baohua@...nel.org, Liam.Howlett@...cle.com,
zhongling0719@....com, ziy@...dia.com, lorenzo.stoakes@...cle.com
Subject: Re: [PATCH] mm/huge_memory: Fix iterator variable usage after swap()
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@bytedance.com/
Right, in
commit 776bde7caf80f6af72b087cafe7d9f607b14716d
Author: Muchun Song <muchun.song@...ux.dev>
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
Powered by blists - more mailing lists