From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6AB6CC5475B for ; Tue, 12 Mar 2024 00:29:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C61336B0096; Mon, 11 Mar 2024 20:29:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C11A26B0099; Mon, 11 Mar 2024 20:29:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD8996B009B; Mon, 11 Mar 2024 20:29:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 99B266B0096 for ; Mon, 11 Mar 2024 20:29:36 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 65DD21C0FB8 for ; Tue, 12 Mar 2024 00:29:36 +0000 (UTC) X-FDA: 81886503552.02.1EDD8F3 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by imf11.hostedemail.com (Postfix) with ESMTP id 22C1840015 for ; Tue, 12 Mar 2024 00:29:33 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=Hfk03n4l; spf=pass (imf11.hostedemail.com: domain of ying.huang@intel.com designates 192.198.163.10 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710203374; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=dNYDKTXem8sH/QYC61lOkMfy5gPQM9bEDXkVyeDXO/Y=; b=y2tfUK86GY2qtURU3YdWAgltb0+EDMXWuIsoHLjZN9GweAPHD9Mv5CRMpatPEg5S4JZMgw A6z6KKvSle7hJr1e8hwNNidRFQZsNWg6D/E+JqI7C4VXt54n0F6Lz/KyRRZRsofF3XILZ7 pOCGqK7r3doDsmLME6+LFpWa4ClkL4E= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710203374; a=rsa-sha256; cv=none; b=bneFEFolEN5jZ4GMPEMC3S7lYBA4WBfyFTw5nWVyE6p0aVkVod6i/4hkodbLXbzH4EU55o 6JiWlnU5umEb9D3dgMmAiVy5ZMF2BVPHeFEIrASWP2ffD3Il0upEIZagE4Ops0QnJWKSJe nb+zeEi7TtFYXVtkNy2rc2PzyONFGeY= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=Hfk03n4l; spf=pass (imf11.hostedemail.com: domain of ying.huang@intel.com designates 192.198.163.10 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1710203374; x=1741739374; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=sW+dADlfFW3sgJ2Rei1AseXsmQNC6ar3yV5n7ckvMsI=; b=Hfk03n4lh0v+Zup1goPuuC2/iQnV5YReoLSR3IO2XAm5uuhx5VYo0Aik By/fjq8v3xPHZv7/6ujS3dAbjG97c2P5VAYGxJPLENgQnlkfaZw7Jdt5G wB2wdYEjIsHBSpWRZWQIIN3CXsybvMJTcxzUkW5fRO862cHrnL3yP3NVV No/NusLfFb19TQSu7XALo0htey2i178u0S9AauENojEUxam6jjb/dFzu0 Tlhvo76ET7dmAdAAUMmuYRDFTS3oOHOZ+7VuYdr5vM7GI3B4QmTU4x4Zs N4I1QafMAj79y8N+Ld5n3FmfgwEGNHl2zBCuFrLtWAVpvC8Gbv9B1FsDb w==; X-IronPort-AV: E=McAfee;i="6600,9927,11010"; a="16296341" X-IronPort-AV: E=Sophos;i="6.07,117,1708416000"; d="scan'208";a="16296341" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Mar 2024 17:29:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,117,1708416000"; d="scan'208";a="11791884" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Mar 2024 17:29:31 -0700 From: "Huang, Ying" To: Ryan Roberts Cc: Andrew Morton , , , David Hildenbrand , Subject: Re: [PATCH -v3] mm: swap: fix race between free_swap_and_cache() and swapoff() In-Reply-To: <1d27e93b-1c6b-4909-859f-e0756974a640@arm.com> (Ryan Roberts's message of "Mon, 11 Mar 2024 09:44:47 +0000") References: <20240311084426.447164-1-ying.huang@intel.com> <1d27e93b-1c6b-4909-859f-e0756974a640@arm.com> Date: Tue, 12 Mar 2024 08:27:36 +0800 Message-ID: <87v85s47lz.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Queue-Id: 22C1840015 X-Rspam-User: X-Stat-Signature: 6fdoqrqamkbuprb8caa5nyspunu8kqwy X-Rspamd-Server: rspam03 X-HE-Tag: 1710203373-541823 X-HE-Meta: U2FsdGVkX19nttI6yThajqAj6hxrcVrPKAzmIQxTxDUsEzMLoxJCNtZEdpiQGUv7EM38LP5Q99NpEGwJgNkAFuuMqhPRNOmAtenuXKsLRMHSodmAEepjoVDEluIlhqI6FSWqwfCaT1VHPQZXF+7LCwfLe6O+N7JsoBfZrxNA+XKheCiAeicGEMMaej5+bt2jqwYmJwkrH9y0XlmXz5sSKlRtZ7bNdxkIvQt1lhl/2a3KkaQe9jgKzGFvIRu/ng9u2PCBP9wX5p7VcRvtjQfWYqnik7ghkGTYSFP5np08VmQchMyMTmGr+mn0oMDLw5rZSJzEUIM2wlLGRGyr+mhYboDv++6+zivQdTFgeEQF1sqSXLpwTpc4/YfH1XfNZ4skcsZr8plnXgRLF0+6/ezyApJraif6K6he+8SiTEUwrMXkmrtEhKU090hjzE2gJqZfX4qf//52tn1ISWDz7uNcZEniCLzUgZvFKABHhFt+Pg3mpkT2LEQyFsVnzgHmDnq62LRGafdH1zvQbkJLPWPz6vc4mtqD+PHmB8UP3fEhPyTppg2Zt29lvQ5BaYVtyHE9ttgU7Jx5kVHXofYVczMQkumwxfwoesoo+K34FVeW79owlirm/8M7mjLiybMbe4tVbO5Jim3bzAq3QK5uiiHSKs+vNmrNHE0FNxGqGvwV57Fdl9INY0N/2kvQDzw+AOTIQFvcp6kVz+KL/gzRCt8Qz1tt7hP8hvoocCEhwBRJcth54sJwzY7VtMX713EsW9+sKTq7K/rxbc4J0A+7lBZDM5qWkNhD87lMRYJDcZLZHSNgp++7ZT8V74/M5qC1ZCGOVY+6GcSxzLfNgumXYpW8SS5tfuwqjFwQ/vXGextMlkm+V8+KBeB6u5lRJcj5LvqVqwsXUqzRQAILnAT91iKNLWTKsDSUcxYk0uhF6uxXLG+K3IDic7Y9RWcVJpGF8OIdkjfC0hJ9aAXrYhL/1B9 dA5eUha0 MPv1SN9fnSBRVWZiHW2y9tB+Zlw6nIn8rDacJaRfJWUXdjEznWQ1Bh3zQXZ4g291PCFAJ65dYrOUU2UG80G+X6Lhi9WvPFZcTy5xLjsECPSr75E0u3M7Z27R6TkEKnIikrX/cuFNdu94RPn2VBqU0EtpuI7kjU+MYf9J/qj9+5fU9pEq5drGeWGvg+XyrYQRf0SnGCrUGzJa/aeCDK1qMiTMdWCrxCZX96/HoLT0OAoJm0xq+NsRwGcdBbadKjBnEMJAAuFX1zh/1t4vWeWZR8K5szAz1Ih5L3FByXeFlX8Tr12Eryxcm0N3WEeTkcgR1WtEKHdkCHg7xMBnhJDvq+uHoVk31yNPk/X7y+FZTZWYbt/jDdhe8jjNdt0CcDIfu/98CRfXrw+wqoimHbCEY06Dn/FM7hkwSiwNQ X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Ryan Roberts writes: > On 11/03/2024 08:44, Huang Ying wrote: >> From: Ryan Roberts >> >> There was previously a theoretical window where swapoff() could run and >> teardown a swap_info_struct while a call to free_swap_and_cache() was >> running in another thread. This could cause, amongst other bad >> possibilities, swap_page_trans_huge_swapped() (called by >> free_swap_and_cache()) to access the freed memory for swap_map. >> >> This is a theoretical problem and I haven't been able to provoke it from a >> test case. But there has been agreement based on code review that this is >> possible (see link below). >> >> Fix it by using get_swap_device()/put_swap_device(), which will stall >> swapoff(). There was an extra check in _swap_info_get() to confirm that >> the swap entry was not free. This isn't present in get_swap_device() >> because it doesn't make sense in general due to the race between getting >> the reference and swapoff. So I've added an equivalent check directly in >> free_swap_and_cache(). >> >> Details of how to provoke one possible issue: >> >> --8<----- >> >> CPU0 CPU1 >> ---- ---- >> shmem_undo_range >> shmem_free_swap >> xa_cmpxchg_irq >> free_swap_and_cache >> __swap_entry_free >> /* swap_count() become 0 */ >> swapoff >> try_to_unuse >> shmem_unuse /* cannot find swap entry */ >> find_next_to_unuse >> filemap_get_folio >> folio_free_swap >> /* remove swap cache */ >> /* free si->swap_map[] */ >> swap_page_trans_huge_swapped <-- access freed si->swap_map !!! >> >> --8<----- >> >> Link: https://lkml.kernel.org/r/20240306140356.3974886-1-ryan.roberts@arm.com >> Closes: https://lore.kernel.org/linux-mm/8734t27awd.fsf@yhuang6-desk2.ccr.corp.intel.com/ >> Signed-off-by: Ryan Roberts >> Signed-off-by: "Huang, Ying" [patch description] >> Cc: David Hildenbrand >> Cc: >> Signed-off-by: Andrew Morton >> --- >> Hi, Andrew, >> >> If it's not too late. Please replace v2 of this patch in mm-stable >> with this version. > > Thanks for sorting this out, Huang, Ying! I saw your note asking if I could do > it, and it was on my list, but I've been busy debugging other urgent issues in > mm-stable. That should be solved now so unblocks me finishing the testing on my > large folios swap-out v4 series. Hopefully that will be incomming in the next > couple of days. You are welcome! > You did previously suggest you wanted some comments around synchronise_rcu() in > swapoff(), but I don't see those here. I don't think that should hold this up > though. I will send a separate patch for that, including comments for get_swap_device() and around synchronize_rcu() in swapoff(). -- Best Regards, Huang, Ying > Thanks, > Ryan > > >> >> Changes since v2: >> >> - Remove comments for get_swap_device() because it's not correct. >> - Revised patch description about the race condition description. >> >> Changes since v1: >> >> - Added comments for get_swap_device() as suggested by David >> - Moved check that swap entry is not free from get_swap_device() to >> free_swap_and_cache() since there are some paths that legitimately call with >> a free offset. >> >> Best Regards, >> Huang, Ying >> >> mm/swapfile.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/mm/swapfile.c b/mm/swapfile.c >> index 2b3a2d85e350..9e0691276f5e 100644 >> --- a/mm/swapfile.c >> +++ b/mm/swapfile.c >> @@ -1609,13 +1609,19 @@ int free_swap_and_cache(swp_entry_t entry) >> if (non_swap_entry(entry)) >> return 1; >> >> - p = _swap_info_get(entry); >> + p = get_swap_device(entry); >> if (p) { >> + if (WARN_ON(data_race(!p->swap_map[swp_offset(entry)]))) { >> + put_swap_device(p); >> + return 0; >> + } >> + >> count = __swap_entry_free(p, entry); >> if (count == SWAP_HAS_CACHE && >> !swap_page_trans_huge_swapped(p, entry)) >> __try_to_reclaim_swap(p, swp_offset(entry), >> TTRS_UNMAPPED | TTRS_FULL); >> + put_swap_device(p); >> } >> return p != NULL; >> }