Received: by 2002:ab2:1689:0:b0:1f7:5705:b850 with SMTP id d9csp2062525lqa; Tue, 30 Apr 2024 07:18:50 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCUr7IzUKloWv6QnOiPAwTpui85K/fg9dUmp/AjZo+TQXGIgaXbXe/nMyIXAjOl8MuLpwEUo5dfus7qEgU4nMQ+kBnvS73TMpHT4Xev73Q== X-Google-Smtp-Source: AGHT+IGmS6D4BO6XRyu4oF/lu0fb3VYaivEeYg451Sw7WexrdFFilIUys2lpCf5TVgEsSD0EmnV+ X-Received: by 2002:a19:7714:0:b0:518:c89c:e846 with SMTP id s20-20020a197714000000b00518c89ce846mr8128208lfc.56.1714486730041; Tue, 30 Apr 2024 07:18:50 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1714486730; cv=pass; d=google.com; s=arc-20160816; b=rPJTtF1jK4ovtumxlx9a7vgFFEZSvqfBoMQt0QbZo9qVTZ560/Sr4uTqbEAj74yvgU kAU3MaxHtHW2PjfehQHrtfaX6bcxcRujRUjwWwjRYKGsB9fhtt9FmzF5wcW72ALYXbu+ 0Q0b8DBKLh1jVpW0/etLKLhXEa+YLT/kGQcTr1/oiDoupMzUJdBcIrTbVJK+z7rlcjip znXT8RKGIBbLp7hbjjfJlYZa2zNSrAaj3aMTLwPQrY0PS6tgb4e4c06iarQWPIQSGF0v O3fe92jKSzokk0OdYAQA/Wa6hpnOPiM243+plamFftbZppUxBL46x5qk1rl0ThloppXu vQyA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:message-id; bh=HL0YKRA/vdJhdntVZlq9QbCb8WfesyMDCwwpKopGRo0=; fh=SM60ni9lnmdbGGS2hEy+NRHy3+Xv1wegpEV9WAa9RaA=; b=cT+hzs8iMHtU7H8XqXqPWQO5O9VhxPFyssNzngTfTI5LnNaEYMiywHrnRvFtC5QuIb C+dDZAZ1gQUzIzY5MfkzbPJ1aeLF43YLSMbug4mj16vKLXKQq6i12WQ7FYaySwv33ITs jq8V+ARTfp1V+z5sExnHsb28q98Heub0q7FcrbPU/1EjcBbElxzuKdFOIUS5coDTXVj0 1cB6Rt1x6bw7W7ct1Nv5tdbJ8URRkXP+cH0ARtUXNqZC0jXlhZD82tysnLf95cmp4Rf5 u8hIdzJ9XH8/jzPrbQV+jrF7K0BfCGn/3MeycMe+ZBsz7Ys6ovMoYFBUnf10gkU9qb2a KwuA==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=arm.com dmarc=pass fromdomain=arm.com); spf=pass (google.com: domain of linux-kernel+bounces-164093-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-164093-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id r2-20020a50c002000000b005726c8689a1si4211001edb.211.2024.04.30.07.18.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Apr 2024 07:18:50 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-164093-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=arm.com dmarc=pass fromdomain=arm.com); spf=pass (google.com: domain of linux-kernel+bounces-164093-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-164093-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 9DA3C1F22245 for ; Tue, 30 Apr 2024 14:18:49 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 1DA3D178CCF; Tue, 30 Apr 2024 14:04:38 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id BAE7C17BB1E; Tue, 30 Apr 2024 14:04:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714485877; cv=none; b=b2Q15wkVl6YNK4mqoNrz60YiCbKHt+wDAV/Ptvk5jcPYy/oquoXxOlok0e72nb1uAXtu9ig6dMFVzzSKPVTEb9joyWwlHaBA+a2cPj38Cf2KG84RIZnKW21zrfapJz3mD3GgtkPoC09xFcFhVXv5X+oe1tHnMO/STE1er+3tOSQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714485877; c=relaxed/simple; bh=hIs71bhG4+cflhzz5GMNftsXvNAdBV+yukiIRd9yUsg=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=FIwTAi8hapNAN7lSmrZibv/86csh8jEMaCM5gk7/bAxA0UnqnUTDgfHWuvSox86R/QyEOYyWp5zNeAhmKrIoZLGQuiY2XCMsIaOfCBAQzoZJ13fStFBEeTdMhre9Z3ocNSpyAU7I2ZoAEkNVxmCmvAlVOnVzaIAnqFQWp2kYh4E= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 837FC2F4; Tue, 30 Apr 2024 07:05:01 -0700 (PDT) Received: from [10.1.38.140] (XHFQ2J9959.cambridge.arm.com [10.1.38.140]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B13F13F793; Tue, 30 Apr 2024 07:04:33 -0700 (PDT) Message-ID: <41a83b7a-17e0-469d-bec4-10ebfff4ef57@arm.com> Date: Tue, 30 Apr 2024 15:04:32 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] arm64/mm: pmd_mkinvalid() must handle swap pmds Content-Language: en-GB To: Will Deacon 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 References: <20240430133138.732088-1-ryan.roberts@arm.com> <20240430135534.GA14069@willie-the-truck> From: Ryan Roberts In-Reply-To: <20240430135534.GA14069@willie-the-truck> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 30/04/2024 14:55, Will Deacon wrote: > 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 Thanks > > But it might be worth splitting the tests from the fix to make backporting > easier. Yes good point. I'll leave this hanging for today to see if any more comments come in, and will re-post tomorrow as 2 patches. I assume we need to go fast to catch 6.9. > > Catalin -- I assume you'll pick this up, but please shout if you want me > to take it instead. > > Will