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 B7F08EB64DD for ; Mon, 3 Jul 2023 08:54:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 08D168E0099; Mon, 3 Jul 2023 04:54:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 03DA08E007C; Mon, 3 Jul 2023 04:54:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E467F8E0099; Mon, 3 Jul 2023 04:53:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id D64C18E007C for ; Mon, 3 Jul 2023 04:53:59 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 998E21C8780 for ; Mon, 3 Jul 2023 08:53:59 +0000 (UTC) X-FDA: 80969688198.05.70FF18A Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf28.hostedemail.com (Postfix) with ESMTP id E14B8C0018 for ; Mon, 3 Jul 2023 08:53:55 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf28.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688374436; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WBhoBBx3iLWPPQttbV99j2CksQvWTQ45ukXMmdKNxKc=; b=S9/0Zh3qe9l/4TWxb0zC1djih69YhGTxy2zxQYkHklzQkFMX/IxsX9reraHrScYupRYeFN JUSHF+RjDCbDC+6RDfBdETTSN/xGh7zjUEYN+y67k0InTh4z+mt3QhrtRhILfr9pfyGliZ uUGvLIgTCyvImfZwBs8B3kgl+nJxeKA= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf28.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688374436; a=rsa-sha256; cv=none; b=aOhvjB/rsuDOZ0ZrZuIwNysx4pitoZANzjaYLutfjVtypETX8NQj9m/f5i4fUpRb5WxEnS rlyGqz3Oeo2S3vWUJutKgg5mWzMg2L8VXHYP5FXdVHbhzAk2+pPQjYUgeSWiyg7K/3f85/ kGNsXcrAffNBKtH6WJVbHgCSZBfpGmU= 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 6A04C1FB; Mon, 3 Jul 2023 01:54:37 -0700 (PDT) Received: from [10.57.76.103] (unknown [10.57.76.103]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C8B1F3F762; Mon, 3 Jul 2023 01:53:47 -0700 (PDT) Message-ID: <8b2df98c-f3d4-a1ab-b3c0-ed456da6f404@arm.com> Date: Mon, 3 Jul 2023 09:53:45 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v2] mm/hugetlb.c: fix a bug within a BUG(): inconsistent pte comparison To: John Hubbard , Andrew Morton Cc: LKML , linux-mm@kvack.org, James Houghton , Muchun Song , Adrian Hunter , Al Viro , Alex Williamson , Alexander Potapenko , Alexander Shishkin , Andrey Konovalov , Andrey Ryabinin , Christian Brauner , Christoph Hellwig , Daniel Vetter , Dave Airlie , Dimitri Sivanich , Dmitry Vyukov , Ian Rogers , Jason Gunthorpe , Jiri Olsa , Johannes Weiner , "Kirill A . Shutemov" , Lorenzo Stoakes , Mark Rutland , Matthew Wilcox , Miaohe Lin , Michal Hocko , Mike Kravetz , Mike Rapoport , Namhyung Kim , Naoya Horiguchi , Oleksandr Tyshchenko , Pavel Tatashin , Roman Gushchin , SeongJae Park , Shakeel Butt , Uladzislau Rezki , Vincenzo Frascino , Yu Zhao References: <20230701010442.2041858-1-jhubbard@nvidia.com> From: Ryan Roberts In-Reply-To: <20230701010442.2041858-1-jhubbard@nvidia.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: E14B8C0018 X-Stat-Signature: 9p64fgqn555mma9p6m7m7oeg34ft3i3h X-Rspam-User: X-HE-Tag: 1688374435-381497 X-HE-Meta: U2FsdGVkX18Fr3kYNxD9We59FuitG5IOKo7bKYiEVcIY+gsQdCkR4MQy9ImB06MKXojmqGL+V+p8SDo8B/TO6a7PnelBZ/Mi3EQg3i2pWqkuyDFwoeAvvPgluvWfcCLAM1rRO5NeoMs5WYAZvwHD5Db2isSUluRfQAUM9/YcSIDAHxNgbV9HkX9P+D+iOeC0tbf1TROxTDdcjEnPpO/sFyRiApmcj6/sxIR0M6PTI9ml3xQrU/LL8xEvRTzk+WrqJHqjgaC0kSPoZY3/LwzWY9cyXLbwGbHtzfi53Z4t+gSxW9VYLnlZkldqbkyw3DpNfV298RK5nJGe0kyDaSD+BxKncXODFMwLO224JCT77yadpRa5vCYm6hB1gz5/ASuuhD5R7JbEp+jmS8hESMLXWZOh0+TQHbDy/maMVmRkHLbsfODydKuakthUrP6viPdUSvkqfhk3uBuMy1Vr/NJuak9guzpw6eVyuI3BHNTNiomkMjbB0l7+zos9nkCYKCihPPtzuNx7FUvA57KSKAQ3x/Pxed+yYiY1mjIDyiQe5vdvwJf5GxF7+8zp9v1q7IzDGrOAi7DakhAcQWeS8+SFYZhda75S9VhyDIpZcaKtPO+FN5kBDmTHrPHwi8dEbrM7WNm3E85IQN+jzg+SQsJ6SIaMuvIO4tealwb8XdMvydpBXf4LvuP++7n+3ml09sE1tiarzhrmomTFDx7VAQad+djkMBBowBGfaQpnRxqxSIcZLxchUaPAtbPZ8+dvgUvVA2v5+Ym+hdy/ubxHPW3Suv+V2pvqi+XHKzlqn2X2EcaXGuNG7k63FSWEcL+ZnOM6hDHAYJG5pEN6AcPVtuSemoz+E8+6o0pRjQOwVQ7OJbOsiOpcOptOCLrvNBHksZLcsuIhIpPt8x18WgEJnSSg2GRnRCqWVPzqUbtm1KKXbk4t7U1zRf/BAsxDuJocBmmOvW4G02EXMgLvJ8eWwO0 HI0ddDrj +qcoZZjytdUOAoBzTjPnjRFcgjqTxIzyquFRkdRZpgxzo0UshY062E4D320a4S16a0xJa2BAMTP/xja5psfW66xXrtgih+ovdQRtXBpZRKEoRAqqoJ2QA61YaUq2ecNAPX2wkdn5c9uv7om5VAnWlU2scrmgu6/Qny74yc64xo02Rg0w2HKLqt+ZaCtLWEL7WcDA86HwAyx2qwDGdq9HH/g/aOx/3zZ3BEyLIVbVZUpyhPpyLbrJadyM1D8PK8j13pX8YJZdJdVFsV0ejYa/JFax/irhaUvngGQf7v1fFPMXLw5LzyJ8MDFrXM7X2SCn0jurNm3dsioub7XtV8wIx4t4F0FYIz+Mb2UDM4wfAETsTAfnXUVhlYnxqyJoFLqN6aHE2u/JWYupUxZpC2+6ZEZg8JJeqlUWIMGQg6oacD5DuJ2N22ioJToQoLJYevFQ8Y1O1Pxoxgtiilt2LK7BAoyHLyXBcN4VCj5NrO8EmS/Ww7nO1xRDZjpmTEsub1yHG/HnGp/zTbLenzv+CckstG9v/A5dZ7a06M76Ir9F8qogQoBW8UvVEc6ETrnP8Un9fC9QWiBq7CVxmGkiQ0FcB01bkC+AseJ/cTlTo/8lp0Taoyu4QxzeIFNBgKqYUFnhelYsgJDtnDEo1EYzChVLUoJKIPcY3oTEfYWxsCHzcYS6uKA5JIBsFifqTvniL+B4EBTXT3dXjqNvFo0yO15zjGrUY8LHfAhD6UYkCJOfFNdYGW9S0AaMKwoswjlaP09L1A7BbQLyquc4PQZvcYu4gg7AZTDBoX9McDAUCoaiRPxjBDQ/NtkPU1RB/fZFdXlaJ1HTIWbCWpdBNPQvgAx82ezkErGfDykn0pEI7QyvxB4HE6jhHHTTP7hRnZR6ZZXpuFwndkYAleFxscoaJg4RFh6P+FBCBlekj1SrAyvZPnqniQY6Nq79HB0rggOLkIJKwcATuaYJIq4G46gFo/TuZOEd1613n mWdYAWaV yLVyTN2GViMY4xX+v7Lx22yz9kbn22HUuLxkb8fZznI= 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: On 01/07/2023 02:04, John Hubbard wrote: > The following crash happens for me when running the -mm selftests > (below). Specifically, it happens while running the uffd-stress > subtests: > > kernel BUG at mm/hugetlb.c:7249! > invalid opcode: 0000 [#1] PREEMPT SMP NOPTI > CPU: 0 PID: 3238 Comm: uffd-stress Not tainted 6.4.0-hubbard-github+ #109 > Hardware name: ASUS X299-A/PRIME X299-A, BIOS 1503 08/03/2018 > RIP: 0010:huge_pte_alloc+0x12c/0x1a0 > ... > Call Trace: > > ? __die_body+0x63/0xb0 > ? die+0x9f/0xc0 > ? do_trap+0xab/0x180 > ? huge_pte_alloc+0x12c/0x1a0 > ? do_error_trap+0xc6/0x110 > ? huge_pte_alloc+0x12c/0x1a0 > ? handle_invalid_op+0x2c/0x40 > ? huge_pte_alloc+0x12c/0x1a0 > ? exc_invalid_op+0x33/0x50 > ? asm_exc_invalid_op+0x16/0x20 > ? __pfx_put_prev_task_idle+0x10/0x10 > ? huge_pte_alloc+0x12c/0x1a0 > hugetlb_fault+0x1a3/0x1120 > ? finish_task_switch+0xb3/0x2a0 > ? lock_is_held_type+0xdb/0x150 > handle_mm_fault+0xb8a/0xd40 > ? find_vma+0x5d/0xa0 > do_user_addr_fault+0x257/0x5d0 > exc_page_fault+0x7b/0x1f0 > asm_exc_page_fault+0x22/0x30 > > That happens because a BUG() statement in huge_pte_alloc() attempts to > check that a pte, if present, is a hugetlb pte, but it does so in a > non-lockless-safe manner that leads to a false BUG() report. > > We got here due to a couple of bugs, each of which by itself was not > quite enough to cause a problem: > > First of all, before commit c33c794828f2("mm: ptep_get() conversion"), > the BUG() statement in huge_pte_alloc() was itself fragile: it relied > upon compiler behavior to only read the pte once, despite using it twice > in the same conditional. > > Next, commit c33c794828f2 ("mm: ptep_get() conversion") broke that > delicate situation, by causing all direct pte reads to be done via > READ_ONCE(). And so READ_ONCE() got called twice within the same BUG() > conditional, leading to comparing (potentially, occasionally) different > versions of the pte, and thus to false BUG() reports. > > Fix this by taking a single snapshot of the pte before using it in the > BUG conditional. > > Now, that commit is only partially to blame here but, people doing > bisections will invariably land there, so this will help them find a fix > for a real crash. And also, the previous behavior was unlikely to ever > expose this bug--it was fragile, yet not actually broken. > > So that's why I chose this commit for the Fixes tag, rather than the > commit that created the original BUG() statement. > > Fixes: c33c794828f2 ("mm: ptep_get() conversion") > Acked-by: James Houghton > Acked-by: Muchun Song > Cc: Adrian Hunter > Cc: Al Viro > Cc: Alex Williamson > Cc: Alexander Potapenko > Cc: Alexander Shishkin > Cc: Andrew Morton > Cc: Andrey Konovalov > Cc: Andrey Ryabinin > Cc: Christian Brauner > Cc: Christoph Hellwig > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: Dimitri Sivanich > Cc: Dmitry Vyukov > Cc: Ian Rogers > Cc: Jason Gunthorpe > Cc: Jiri Olsa > Cc: Johannes Weiner > Cc: Kirill A. Shutemov > Cc: Lorenzo Stoakes > Cc: Mark Rutland > Cc: Matthew Wilcox > Cc: Miaohe Lin > Cc: Michal Hocko > Cc: Mike Kravetz > Cc: Mike Rapoport (IBM) > Cc: Namhyung Kim > Cc: Naoya Horiguchi > Cc: Oleksandr Tyshchenko > Cc: Pavel Tatashin > Cc: Roman Gushchin > Cc: Ryan Roberts > Cc: SeongJae Park > Cc: Shakeel Butt > Cc: Uladzislau Rezki (Sony) > Cc: Vincenzo Frascino > Cc: Yu Zhao > Signed-off-by: John Hubbard Reviewed-by: Ryan Roberts > --- > > Changes since v1: > > Added Acked-by's. > > Fixed as per Ryan Roberts (thanks!): changed to ptep_get_lockless(). > > > mm/hugetlb.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index bce28cca73a1..64a3239b6407 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -7246,7 +7246,12 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma, > pte = (pte_t *)pmd_alloc(mm, pud, addr); > } > } > - BUG_ON(pte && pte_present(ptep_get(pte)) && !pte_huge(ptep_get(pte))); > + > + if (pte) { > + pte_t pteval = ptep_get_lockless(pte); > + > + BUG_ON(pte_present(pteval) && !pte_huge(pteval)); > + } > > return pte; > } > > base-commit: bf1fa6f15553df04f2bdd06190ccd5f388ab0777