Received-SPF: pass (google.com: domain of linux-kernel+bounces-161081-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Zi Yan <ziy@nvidia.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>,
 Anshuman Khandual <anshuman.khandual@arm.com>,
 Andrew Morton <akpm@linux-foundation.org>,
 "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>, linux-mm@kvack.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] mm: Fix race between __split_huge_pmd_locked() and
 GUP-fast
Date: Sat, 27 Apr 2024 11:07:25 -0400
Message-ID: <6F2BB00A-DBCD-4482-B16E-B71A02847F0D@nvidia.com>
In-Reply-To: <1734de6e-1bcd-492e-b07b-9ad712967e1d@nvidia.com>
References: <20240425170704.3379492-1-ryan.roberts@arm.com>
 <39d0236c-4297-4001-bc7e-f516bc674f7b@arm.com>
 <87df74b7-2352-4ad6-b9f9-baed9291cbed@arm.com>
 <80148272-208A-4306-A8EC-98C79BBBDCA0@nvidia.com>
 <D0E6B5DC-6206-408C-81D1-C9B0A85E1D0E@nvidia.com>
 <1734de6e-1bcd-492e-b07b-9ad712967e1d@nvidia.com>
Content-Type: multipart/signed;
 boundary="=_MailMate_1FD05359-9965-47F6-8621-EF7035B67727_=";
 micalg=pgp-sha512; protocol="application/pgp-signature"
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?oQkPSHIy7BMIz3ph26gA/GEZEWak2vnMXKxXhK1A7w2SKfi48ZeSUWMRCqhy?=
 =?us-ascii?Q?s6C8njJNPcDCRy26Vdzx4WERWxgSTIgPGggQTiIx80wMqBGnVjszB+lOh6EG?=
 =?us-ascii?Q?VKxUVnezk2fust3soKqlc2Erv4z17fmsX++ZdrK3Mhp1c2qyjG4WJkrBZDqv?=
 =?us-ascii?Q?ygOnOYVdrpEWgsYFjlSqH8dsQle4Q+ZBgICk6+rk3FcimD2NG1/ic9mu/yK4?=
 =?us-ascii?Q?x1jyTd8nddzUFVZOoysJkzsY2z7ncdmwE70PBw5rgl3A9t+Y/AJk0gQ27VF4?=
 =?us-ascii?Q?GR2SCPEQI7Uatrs7lpITsbB9xeKm72lP6SDwmNt7o6tbjtL3ZT7jn7CFDqYG?=
 =?us-ascii?Q?/o21+f5AulUAhq2Pxa+XqMfQBff01YI7YjDPpM5VNnBhQZPK11/yGaBJWRjd?=
 =?us-ascii?Q?8/Fzv6iH2bLUfDZCemPVcc9RBdMXyuQ4m97cN7YQNnq437rW0Q/IIjH0S8G5?=
 =?us-ascii?Q?yDOwrLv4xZwvcXNity7AjXxUxtfd/1/UnrlOt4KBTtzP2SA3bTe8U92nXNtS?=
 =?us-ascii?Q?Ni6ZaSToSfN7et1/IaY1mcb6CfzIEaK71XQeiQXKOsJ6rKyOy8d1ZUzhOwWB?=
 =?us-ascii?Q?ul/JgJ0Un3VHMHULdKNFGxMg+qy5kyZauwDtwTcCg7Qd+J5hQl3FgZw3CiKV?=
 =?us-ascii?Q?/fWn0iBedHkL2pG1X7MJxe+UF/nhfYQ7+hikT5gqC5iq8TtGTeVJNAYZyAXQ?=
 =?us-ascii?Q?/BIrwKMUuM74887UrBWUt0d6w7+uXEA5LN+j8njLlbKGYLc2oNZfbQ54vxDT?=
 =?us-ascii?Q?gBVU2ln+hNlKhb1r8JnTehnKyP5ULOMtBA+GfIEpDWLhOqXPYWXSPU8q1Ulw?=
 =?us-ascii?Q?TW4ln0QhSf8iLxlcChr9KQhAUjey6uHrXHBAmzgZqaPMgrZK4RhMhzBIDd/Y?=
 =?us-ascii?Q?/Qnl0xcxQrE9HByz6knBJYKVLUY5U/4m40MwQAuwLar8JsyiB8BmaWfjHtD2?=
 =?us-ascii?Q?dBbKFLkEgibaLMVRomX5DBN20LoFR/QYszVPbF2kXDN7oo7baI17U2RsX+1m?=
 =?us-ascii?Q?hyj+dHvjllAW549UpWxdmLUWaqBTKVbUl32AqGo9RjwUrEZ7IyNc6FPBj/2U?=
 =?us-ascii?Q?rwXbrwR6XPGLHGjXSuBdUvKS2Q+T3nxbDy8w4KPg4F/agPYGIxZx4ngVs+9b?=
 =?us-ascii?Q?HNNNqTWTs7ef0UkkQH57cJy+YbKqm18Mi8fbtZkNQfeynMW86IO7++ijEekr?=
 =?us-ascii?Q?JA19qXDtt2Q6OF3sIGJdYw4XjYKWUPuEI7hK78diTb+zvdtiT5/EnAddRoQs?=
 =?us-ascii?Q?TOhXr4hTH/6zZ9qA59GjuXqU2TpBxYyO8CJz4b7iV/pYnooO1u2GtsZrkkLY?=
 =?us-ascii?Q?x5rY5zabAIGht6VAcEP/zMcSavNTXDaE8G8/V4l7wMDvCAW02bc4H9HKlgw8?=
 =?us-ascii?Q?n8RmyHhBXySzXszVZeXByWyl5APg6c4R1d/GW4F2wi8o+fElMIkWF96kckD8?=
 =?us-ascii?Q?7IIq+sN1cOzPnVu/Fb1JoSSDFaNTsDsBkBu+IDUbKYqyh7r7Jz0lzk95njHj?=
 =?us-ascii?Q?o9Nu6JW7Lx6Z50pDfXkUBgekWVzqrTLLsJEZVsZLYgrqB7IIXCR1ChPQtpnO?=
 =?us-ascii?Q?dpq6VJiIyM1RZ018oeQ=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6da377c5-a0fc-45de-76c0-08dc66cbc18b
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Apr 2024 15:07:29.2829
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3MvARsXyLhUzzXtdIh6JoKW4dkH1TZwyuh7ycljICom9XI6btsWkxKfUbVRyI536
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB9086

--=_MailMate_1FD05359-9965-47F6-8621-EF7035B67727_=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On 27 Apr 2024, at 0:25, John Hubbard wrote:

> On 4/26/24 7:53 AM, Zi Yan wrote:
>
> Hi Zi (and Ryan)!
>
>>>>>> lockless pgtable walker could see the migration entry pmd in this =
state
>>>>>> and start interpretting the fields as if it were present, leading =
to
>>>>>> BadThings (TM). GUP-fast appears to be one such lockless pgtable w=
alker.
>>>>>
>>>>> Could you please explain how bad things might happen ?
>>>>
>>>> See 2 places where pmdp_get_lockless() is called in gup.c, without t=
he PTL.
>>>> These could both return the swap pte for which pmd_mkinvalid() has b=
een called.
>>>> In both cases, this would lead to the pmd_present() check eroneously=
 returning
>>>> true, eventually causing incorrect interpretation of the pte fields.=
 e.g.:
>>>>
>>>> gup_pmd_range()
>>>>    pmd_t pmd =3D pmdp_get_lockless(pmdp);
>>>>    gup_huge_pmd(pmd, ...)
>>>>      page =3D nth_page(pmd_page(orig), (addr & ~PMD_MASK) >> PAGE_SH=
IFT);
>>>>
>>>> page is guff.
>>>>
>>>> Let me know what you think!
>>
>> Add JohnH to check GUP code.
>
> Ryan is correct about this behavior.
>
> By the way, remember that gup is not the only lockless page table
> walker: there is also the CPU hardware itself, which inconveniently
> refuses to bother with taking page table locks. :)
>
> So if we have code that can make a non-present PTE appear to be present=

> to any of these page walkers, whether software or hardware, it's a
> definitely Not Good and will lead directly to bugs.

This issue does not bother hardware, because the PTE_VALID/PMD_SECT_VALID=

is always unset and hardware always sees this PMD as invalid. It is a pur=
e
software issue, since for THP splitting, we do not want hardware to acces=
s
the page but still allow kernel to user pmd_page() to get the pfn, so
pmd_present() returns true even if PTE_VALID/PMD_SECT_VALID is unset by
setting and checking PMD_PRESENT_INVALID bit. pmd_mkinvalid() sets
PMD_PRESENT_INVALID, turning a migration entry from !pmd_present() to
pmd_present(), while it is always a invalid PMD to hardware.

>
> Since I had to study this patch and discussion a bit in order to
> respond, I'll go ahead and also reply to the original patch with review=

> comments.
>
>
> thanks,
> -- =

> John Hubbard
> NVIDIA


--
Best Regards,
Yan, Zi

--=_MailMate_1FD05359-9965-47F6-8621-EF7035B67727_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQJDBAEBCgAtFiEE6rR4j8RuQ2XmaZol4n+egRQHKFQFAmYtFK4PHHppeUBudmlk
aWEuY29tAAoJEOJ/noEUByhU7JYP/RIZ/7hML6LeEGHYZEKBJ/7xNKLghDODAmtT
LiFkOtgs6LusbevS4837wxNm4bigQUfiZ9vveWsc2HXeiJEzUqyiKShPny2B8yTW
7MAx79JqTeA/zl0RDYonXrYVXIrAKEab0wwZdlPksaZLwAJ2FGZZAt0LdOKOC7vd
cR2if2BSPWvivwQga6DAJNLZUEqOxJ1VdFvNlpwRSPJUHQRlra2rzJ4tsroyKB6y
VS6cJqVv33aX/EpS7llqZ0CM4LY+FZMLJeM9UNS+nWQTdDUE3mHugerToVAe6Iqx
qQDcsMqarpBTq5zr+wA2XIsbuYwoKHup5yVj2nhGv/5W4cckvNGYm3E+GCmMBiSU
mbP11JKnCsa1lYZnHP6+/2ol7X0eBWSnZtVSzqjSL00fT3BfXoIy5z/wm4H6ddKV
DIgviefFej2yVCINL7sUuTQGH2AUSKaf8Ve2WzMHHr+9rPUV10FWVi2QDC1p4WMH
V2c3ZEp2izTAEMs4w/6hOSVZkyVGOFV4Y2f11BPgKsbE+2pulu7cvfxp396qBKgb
YlPsrUIHrndrFDj1SaqSUFutB1eTirLXU/850YUInMAR0/fpYdSGhQTCiH52kgBs
gI9S0X6I/Z/w9W5sLxhFEH/Azej24CP1cnAXFWiMmyAE79ODvoQ0QMv2enfacYLA
7rSjicdT
=zPD1
-----END PGP SIGNATURE-----

--=_MailMate_1FD05359-9965-47F6-8621-EF7035B67727_=--