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 CEB11C4345F for ; Tue, 30 Apr 2024 13:55:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 60B966B0092; Tue, 30 Apr 2024 09:55:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5BBAD6B0095; Tue, 30 Apr 2024 09:55:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 482C76B0096; Tue, 30 Apr 2024 09:55:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 2C3196B0092 for ; Tue, 30 Apr 2024 09:55:45 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id D50491A0915 for ; Tue, 30 Apr 2024 13:55:44 +0000 (UTC) X-FDA: 82066346208.15.302FC26 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf13.hostedemail.com (Postfix) with ESMTP id 0AB0D20021 for ; Tue, 30 Apr 2024 13:55:42 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=upbKBlQe; spf=pass (imf13.hostedemail.com: domain of will@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=will@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1714485343; 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=Ipm4piExz4fhMUYifzkm+nS9/jbIuJ4dvQJXfLZBrDo=; b=kf0VUujIPD9EZ8RBjin3T9L1BJg9bKmokVblYQBVkil/DQh/jR0ZYEd7RBdFL6u3aVu1kb YzM+H5WbH9o4cE97zx+e6UPc6qyAunNJAuEr9nliSPdEwWHJwsbXmNOKKWRHYUxHf0Ojc6 sL5xu66p+5IpQnHNikmsOVp4NjLhtUk= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=upbKBlQe; spf=pass (imf13.hostedemail.com: domain of will@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=will@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714485343; a=rsa-sha256; cv=none; b=i8TGk56d4aBwubX/MAO1wmmENq7FVjDZjFTOVSwxXN3n2K8jzY7qVq+n7h/rjIa+zudkct gHDPSuZK/2+H2SPdVkxN0axCFiZPrJW50Dv5FQCivFutNMOCEheO/qdnFOyGGzuww/XI7U Aaz6SljLoQ8nk1TDy6OXAG3MOwbVZDU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 722F3615DE; Tue, 30 Apr 2024 13:55:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E7511C2BBFC; Tue, 30 Apr 2024 13:55:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1714485340; bh=M7IVnGFLr+3YdAJm1nNLyT13rNLM67abtp8PgBWVffk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=upbKBlQeFcVEf6/hkL7/jrhL4sEXoRGENpi17fBMgfdnQQmPfzNr7PjoxOS7DiP2j SI4uVLUQ2wJCUZ+tIpUUb6fgU6ftff1BY521OGdLz8hfeIgan08jivABUZATPZ3IQX zOk6/wuUV2yesD6xWCX4s/M4Kjop0/MTKRzDe9gH1SHcKc++juiEHlO6mMqPBDmo2O ONRhxKifKJq6W7hRwEcCBSrXphHbvwWk8GuKkheMsuohSFpKuASD8axHptDRtV/Nxe b5kK/14wIxZR8+mrgflcmIgnVKPCQbfaYiWqFLyqFcOT033Tq2k+3VNckdzFm8rooa ZAiD782xs5alA== Date: Tue, 30 Apr 2024 14:55:34 +0100 From: Will Deacon To: Ryan Roberts Cc: Catalin Marinas , Mark Rutland , Anshuman Khandual , Andrew Morton , Zi Yan , "Aneesh Kumar K.V" , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH v2] arm64/mm: pmd_mkinvalid() must handle swap pmds Message-ID: <20240430135534.GA14069@willie-the-truck> References: <20240430133138.732088-1-ryan.roberts@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240430133138.732088-1-ryan.roberts@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-Stat-Signature: 5xs5mz8bztmfk4fscojs885fty3ify5a X-Rspam-User: X-Rspamd-Queue-Id: 0AB0D20021 X-Rspamd-Server: rspam05 X-HE-Tag: 1714485342-658317 X-HE-Meta: U2FsdGVkX18i7AdSRdWb784K4jqsPq+BeWcd7QhBR69C1u/Sae1rEOx8IuyAmPRcgFE0M+rbR+H42DciIvgY+uuVujUQljXm6iYmQ+8u6uTweiv3npAeOItJ8burxKYaPdXKLvnAepXV4KDguhbvID5xUyQekrn+4wHhNFv0eFdnFciplN3OWF1eSEqRUUYHvtFM/HQ3zo3tL1CJtfCmeHe7pIDPmnjYA6ByU0F1OZMNZDIHACRB9QqohScwq6fMbxWETD9V9BNxuP0faxJclR0cJOsviqdFKHBvTjVyeaEJz0UFi67YX8aZ4Vazy1NuxijG4ZyDX6lHd5QXIigci4NfFwFltYobcCf3YL0tqvZXPqxdZuk4ZFtds21HP06rABM0G/Ko50Ui6yViVyOLJ5+WpbzsaHkrT8hI8ZrKjZjz1Lt0JGyKMqiD19mnPJWz3sVbCCcr9gVJtDSVgxeSRdqTHQwDhdw0tBfPS4Xsl1xGY6XkwIcUl503eY4ZPy1TQUXuamiHrMi18VNg/tOliWYyfgthrlBPaZhvbKmbjNLmPjH0WDaDw0lIROJjtnu19n0su3lVxEZtfDt08NwTKaUvU1Uuzkcyolb+S0Ls+GaGbhUl03m7Q86xzUPXSiQCoKxTurZC+ceacJgsnICA4j+Cj12VAg1x13P2MsJimi2UPWKnLOqA6WA6zx5oBWLO4BcBmj3bnZktmEt8KPJlDGZVkKXJrZmd1Q9z+LS+bRqTLBbX4BkL6AJ3gMYzx0ACG5KQA5l1gegMt2FMxSW9gCTpI+KA19owYMaKF69DAAVVwIK4EoG2CNbdicYHctpLw14XasB4Z7Kko1CRMfsVcE+h5Q9VxdrJBZihlihZLnuiN+MB5+GeiIZN6Pk5GJoTvukf3KC0eM4u0f5akP4Oe8zDqikhKw9OtV8EoLCWFc2w7RLQYzlNNV53cr2QmazLg8BrPAKNIHkwiOeaqEG sne2BM3T HIQggsyyK5Vu5OEoJrQAC5Yp+LtmiY0OtnxKYtfcmkNMUN5B98c5FBTC6He1dbI4OVNYmfXfh71bAA2fyFbpzvp+VT7nJerU8NQE9DB1ZsORrgG9fjlVOZ38K4Hr/qYqmAUISnolicZhOhuXX9iDo2zoq/Hg8Wg9vU74s8KVdXhfol+y5BAdCJxg+yJJj2VsesI60JqUst1PQJo61VB6OJrDVxg3CZoR4uEKXJ4CWVShL9zYBq4rhp8ZuSOX7lHbTqISSSnEn1shINGZXhYvB2+YV59biCvn6/L58IGd7sUn4X13PEfn2JM+tWnkd8grMi06ioOlhuE6aJs+DQp9gmymj0+xupDWqYu4c 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: On Tue, Apr 30, 2024 at 02:31:38PM +0100, Ryan Roberts wrote: > __split_huge_pmd_locked() can be called for a present THP, devmap or > (non-present) migration entry. It calls pmdp_invalidate() > unconditionally on the pmdp and only determines if it is present or not > based on the returned old pmd. > > But arm64's pmd_mkinvalid(), called by pmdp_invalidate(), > unconditionally sets the PMD_PRESENT_INVALID flag, which causes future > pmd_present() calls to return true - even for a swap pmd. Therefore any > lockless pgtable walker could see the migration entry pmd in this state > and start interpretting the fields (e.g. pmd_pfn()) as if it were > present, leading to BadThings (TM). GUP-fast appears to be one such > lockless pgtable walker. > > While the obvious fix is for core-mm to avoid such calls for non-present > pmds (pmdp_invalidate() will also issue TLBI which is not necessary for > this case either), all other arches that implement pmd_mkinvalid() do it > in such a way that it is robust to being called with a non-present pmd. > So it is simpler and safer to make arm64 robust too. This approach means > we can even add tests to debug_vm_pgtable.c to validate the required > behaviour. > > This is a theoretical bug found during code review. I don't have any > test case to trigger it in practice. > > Cc: stable@vger.kernel.org > Fixes: 53fa117bb33c ("arm64/mm: Enable THP migration") > Signed-off-by: Ryan Roberts > --- > > Hi all, > > v1 of this fix [1] took the approach of fixing core-mm to never call > pmdp_invalidate() on a non-present pmd. But Zi Yan highlighted that only arm64 > suffers this problem; all other arches are robust. So his suggestion was to > instead make arm64 robust in the same way and add tests to validate it. Despite > my stated reservations in the context of the v1 discussion, having thought on it > for a bit, I now agree with Zi Yan. Hence this post. > > Andrew has v1 in mm-unstable at the moment, so probably the best thing to do is > remove it from there and have this go in through the arm64 tree? Assuming there > is agreement that this approach is right one. > > This applies on top of v6.9-rc5. Passes all the mm selftests on arm64. > > [1] https://lore.kernel.org/linux-mm/20240425170704.3379492-1-ryan.roberts@arm.com/ > > Thanks, > Ryan > > > arch/arm64/include/asm/pgtable.h | 12 +++++-- > mm/debug_vm_pgtable.c | 61 ++++++++++++++++++++++++++++++++ > 2 files changed, 71 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > index afdd56d26ad7..7d580271a46d 100644 > --- a/arch/arm64/include/asm/pgtable.h > +++ b/arch/arm64/include/asm/pgtable.h > @@ -511,8 +511,16 @@ static inline int pmd_trans_huge(pmd_t pmd) > > static inline pmd_t pmd_mkinvalid(pmd_t pmd) > { > - pmd = set_pmd_bit(pmd, __pgprot(PMD_PRESENT_INVALID)); > - pmd = clear_pmd_bit(pmd, __pgprot(PMD_SECT_VALID)); > + /* > + * If not valid then either we are already present-invalid or we are > + * not-present (i.e. none or swap entry). We must not convert > + * not-present to present-invalid. Unbelievably, the core-mm may call > + * pmd_mkinvalid() for a swap entry and all other arches can handle it. > + */ > + if (pmd_valid(pmd)) { > + pmd = set_pmd_bit(pmd, __pgprot(PMD_PRESENT_INVALID)); > + pmd = clear_pmd_bit(pmd, __pgprot(PMD_SECT_VALID)); > + } > > return pmd; > } Acked-by: Will Deacon But it might be worth splitting the tests from the fix to make backporting easier. Catalin -- I assume you'll pick this up, but please shout if you want me to take it instead. Will