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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C3CE6C433FE for ; Wed, 23 Nov 2022 04:29:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+nu+ASkdw4iZxCMms5FXLVx0SPdio64gdzaWOfuzohc=; b=Z1YeZtWUxFwmwt n+lxG7WoXPXdHqqUy7osWmhvFyhnY+fcMwDPgfFIXzWmw05GmHdUIOgaUjJe0MwnxhmhyZlLdKyAI 8mo9wRSewPFaDEmU/NqyCT4zUlhC9+Ep2CXI8CBZJvJvICGgXqkZ8HNZ7JnoPsAwRooOYbwjX7hDW kfbgBF0D0YG7W3dyFA+50ogCIGdScShGYDxBRLdjv2LJJJ31UTwNu6UQyWlnFyztoy3An0yoMAZNB KmxxnxqMMPyqJ+Tav7Q87eN6Jj0asI2oFj1LOCAoE9t3zAh9Y7xRlje5gIsqavQjoVKHumTNAwutL d0C94NOL7Z5YaQw+9WVw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oxhMT-00Dsfp-Hj; Wed, 23 Nov 2022 04:27:49 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oxhMQ-00DseZ-7Y for linux-arm-kernel@lists.infradead.org; Wed, 23 Nov 2022 04:27:48 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8D9F11FB; Tue, 22 Nov 2022 20:27:48 -0800 (PST) Received: from [10.162.43.6] (unknown [10.162.43.6]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1B0A73F587; Tue, 22 Nov 2022 20:27:39 -0800 (PST) Message-ID: <3e6edbea-d5ac-1d2e-737d-ceb7c7e7d7fe@arm.com> Date: Wed, 23 Nov 2022 09:57:37 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH] arm64/mm: Intercept pfn changes in set_pte_at() Content-Language: en-US To: Mark Rutland , Will Deacon Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, Andrew Morton , linux-kernel@vger.kernel.org References: <20221116031001.292236-1-anshuman.khandual@arm.com> <20221118141317.GF4046@willie-the-truck> <879e561c-e834-196c-b9c5-6e44ac2c0296@arm.com> <20221122095748.GA19471@willie-the-truck> From: Anshuman Khandual In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221122_202746_394968_F8DB967B X-CRM114-Status: GOOD ( 27.51 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 11/22/22 16:41, Mark Rutland wrote: > On Tue, Nov 22, 2022 at 09:57:49AM +0000, Will Deacon wrote: >> On Tue, Nov 22, 2022 at 01:43:17PM +0530, Anshuman Khandual wrote: >>> >>> >>> On 11/18/22 19:43, Will Deacon wrote: >>>> On Wed, Nov 16, 2022 at 08:40:01AM +0530, Anshuman Khandual wrote: >>>>> Changing pfn on a user page table mapped entry, without first going through >>>>> break-before-make (BBM) procedure is unsafe. This just updates set_pte_at() >>>>> to intercept such changes, via an updated pgattr_change_is_safe(). This new >>>>> check happens via __check_racy_pte_update(), which has now been renamed as >>>>> __check_safe_pte_update(). >>>>> >>>>> Cc: Catalin Marinas >>>>> Cc: Will Deacon >>>>> Cc: Mark Rutland >>>>> Cc: Andrew Morton >>>>> Cc: linux-arm-kernel@lists.infradead.org >>>>> Cc: linux-kernel@vger.kernel.org >>>>> Signed-off-by: Anshuman Khandual >>>>> --- >>>>> This applies on v6.1-rc4 >>>>> >>>>> arch/arm64/include/asm/pgtable.h | 8 ++++++-- >>>>> arch/arm64/mm/mmu.c | 8 +++++++- >>>>> 2 files changed, 13 insertions(+), 3 deletions(-) >>>> >>>> I remember Mark saying that BBM is sometimes violated by the core code in >>>> cases where the pte isn't actually part of a live pgtable (e.g. if it's on >>>> the stack or part of a newly allocated table). Won't that cause false >>>> positives here? >>> >>> Could you please elaborate ? If the pte is not on a live page table, then >>> pte_valid() will return negative on such entries. So any update there will >>> be safe. I am wondering, how this change will cause false positives which >>> would not have been possible earlier. >> >> I don't think pte_valid() will always return false for these entries. >> Consider, for example, ptes which are valid but which live in a table that >> is not reachable by the MMU. I think this is what Mark had in mind, but it >> would be helpful if he could chime in with the specific example he ran into. > > Yup -- that was the case I had in mind. IIRC I hit that in the past when trying > to do something similar, but I can't recall exactly where that was. I suspect > that was probably to do with page migration or huge page splitting/merging. > > Looking around, at least __split_huge_zero_page_pmd() and > __split_huge_pmd_locked() do something like that, creating a temporary pmd > entry on the stack, populating a table of non-live but valid ptes, then > plumbing it into the real pmd. In both cases i.e __split_huge_zero_page_pmd() and __split_huge_pmd_locked(), the entry is first asserted to be empty via pte_none(), before writing a new value in there. set_pte_at() would still consider such updates safe because pfn_valid(old) will return negative on such entries. VM_BUG_ON(!pte_none(*pte)); set_pte_at(mm, haddr, pte, entry); But if these entries still get updated yet again (while still being inactive) with new pte values, then set_pte_at() would complain for the pfn update on the entry, while being "valid". But is this a viable scenario ? > > We'd need to check that there aren't other cases like that. > Sure, might be some what tricky but anything in particular to be looked into ? I guess if this change gets into a CI system which runs all memory stress tests for long enough with CONFIG_DEBUG_VM enabled, we might get some more clue if there are other similar scenarios possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel