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 for Android: free password hash cracker in your pocket
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ