From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-182.mta0.migadu.com (out-182.mta0.migadu.com [91.218.175.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D27301DDC37 for ; Wed, 15 Apr 2026 09:46:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.182 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776246375; cv=none; b=LcTt+3TyXZIqzrUVDl4mUH6tk+Un/KflgJdHj2RvgkVoTjK3F1j2f5RVPZ+vexxxeLKRHb12fPhwVJLbmQwGMY7a/Ng0kW3y15Tt0iwb+S7yMuSBwu3jaZ4JRK6wmmO2sixTe7JyEmik5Kr1kTEh/MGGXHD6c7k+E8KnjXZlvU0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776246375; c=relaxed/simple; bh=SVBE9xTjODuzuq1SZu4SGvyAci4xv6RKQ9n6RGT3hIg=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=otsqRAn9mi2fO7XmMOZcPcpowdtI1kpmoaOkoRvFOgx+Z6Mp/PNlxxG6E32iEim3EgUzJQFHgaj3e5wUGnt+0snySPd4HtfLpfdli0Y5Wr1bIAfh0JEzO+0l2CqM7rtg3i7wk1cwbPXV6tzm4dRHyCnPWpYIFk+IjVMOOVcZLSM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=QBjm0Q98; arc=none smtp.client-ip=91.218.175.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="QBjm0Q98" Content-Type: text/plain; charset=utf-8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1776246371; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xd8e3J+b1LcawueW4g5SlvWwVt0JRcLEm6Lowqe8HOs=; b=QBjm0Q98krTet/JEVaYxsXi3oRR/HslqU6S8M/huMahtsUrSqju2CsMK51OOb10zGFFiht b2u0G+nNTz3ItgjIOnwsT41ZtHsmuZwONXdX1Fr79WE63a+3itGIICjNzcoPxz4dZ0SVsV qMEIFNVptxhj8mN9y+7WGVtkDGVJfNs= Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.500.181\)) Subject: Re: [PATCH] mm/sparse: Fix race on mem_section->usage in pfn walkers X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song In-Reply-To: Date: Wed, 15 Apr 2026 17:45:30 +0800 Cc: Muchun Song , Andrew Morton , David Hildenbrand , Charan Teja Kalla , Kairui Song , Qi Zheng , Shakeel Butt , Barry Song , Axel Rasmussen , Yuanchu Xie , Wei Xu , Lorenzo Stoakes , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <9E4115F4-5787-495F-BCEE-1E9B1113E9E5@linux.dev> References: <20260415022326.53218-1-songmuchun@bytedance.com> To: Oscar Salvador X-Migadu-Flow: FLOW_OUT > On Apr 15, 2026, at 16:37, Oscar Salvador wrote: >=20 > On Wed, Apr 15, 2026 at 10:23:26AM +0800, Muchun Song wrote: >> When memory is hot-removed, section_deactivate() can tear down >> mem_section->usage while concurrent pfn walkers still inspect the >> subsection map via pfn_section_valid() or pfn_section_first_valid(). >>=20 >> After commit 5ec8e8ea8b77 ("mm/sparsemem: fix race in accessing >> memory_section->usage") converted the teardown to an RCU-based >> scheme, the code still relies on SECTION_HAS_MEM_MAP becoming visible >> to readers before ms->usage is cleared and queued for freeing. >>=20 >> That ordering is not guaranteed. section_deactivate() can clear >> ms->usage and queue kfree_rcu() before another CPU observes the >> SECTION_HAS_MEM_MAP clear. A concurrent pfn walker can therefore see >> valid_section() return true, enter its sched-RCU read-side critical >> section after kfree_rcu() has already been queued, and then = dereference >> a stale ms->usage pointer. >>=20 >> And pfn_to_online_page() can call pfn_section_valid() without its >> own sched-RCU read-side critical section, which has similar problem. >>=20 >> The race looks like this: >>=20 >> compact_zone() memunmap_pages >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> __remove_pages()-> >> sparse_remove_section()-> >> section_deactivate(): >> a) [ Clear = SECTION_HAS_MEM_MAP >> is reordered to b) ] >> kfree_rcu(ms->usage) >> __pageblock_pfn_to_page >> ...... >> pfn_valid(): >> rcu_read_lock_sched() >> valid_section() // return true >> pfn_section_valid() >> [Access ms->usage which is UAF] >> WRITE_ONCE(ms->usage, NULL) >> rcu_read_unlock_sched() b) Clear SECTION_HAS_MEM_MAP >>=20 >> Fix this by using rcu_replace_pointer() when clearing ms->usage in >> section_deactivate(), then it does not rely on the order of clearing >> of SECTION_HAS_MEM_MAP. >=20 > The fix itself does not look too intrusive and I guess it kind of = makes > sense when you think about the ordering issue, so if we want to be > rock solid, why not. You are right. Generally speaking, where RCU is used, the pointer should be cleared first, and then the memory corresponding to the pointer = should be released via kfree_rcu(). Therefore, the correct sequence of use here should be: WRITE_ONCE(ms->usage, NULL); kfree_rcu(ms->usage); The actual code has the order reversed. To prevent such errors, the RCU mechanism provides the rcu_replace_pointer() interface. If you look at = its implementation, this interface is essentially equivalent to the code = sequence mentioned above. > Does it slow down operations a lot? Regarding section_deactivate, I believe there is no functional = difference. >=20 > I would also point out that you rcu-protect pfn_section_valid(). rcu_dereference_sched() is equivalent to the previous READ_ONCE(), but = for RCU-protected resources, it is recommended to use the RCU-specific = interfaces to avoid having to manually account for memory ordering issues. >=20 > Regarding the pfn_to_online_page() race, that is something that every = now > and then pops up, but as David said, we never seen that happening in = the > wild so I guess no one really made the time to look into that. After taking a closer look at commit 5ec8e8ea8b77, it=E2=80=99s clear = that it was intended to resolve a race between __pageblock_pfn_to_page and = section_deactivate. While the issue surfaced this time due to ms->usage, I=E2=80=99m = concerned that even with ms->usage fixed, the underlying race condition in the execution = path still exists. This suggests that subsequent accesses to struct page = might run into similar trouble down the road. That said, I=E2=80=99d love to hear what everyone thinks about whether = this warrants a full fix. Thanks, Muchun. >=20 >=20 >=20 > --=20 > Oscar Salvador > SUSE Labs