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 5CD98EB64D9 for ; Tue, 4 Jul 2023 07:06:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B44CE280060; Tue, 4 Jul 2023 03:06:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF472280049; Tue, 4 Jul 2023 03:06:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E338280060; Tue, 4 Jul 2023 03:06:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 85B3A280049 for ; Tue, 4 Jul 2023 03:06:51 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 4B262A09CA for ; Tue, 4 Jul 2023 07:06:51 +0000 (UTC) X-FDA: 80973047022.07.406B82E Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf14.hostedemail.com (Postfix) with ESMTP id 3299D100017 for ; Tue, 4 Jul 2023 07:06:48 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688454409; a=rsa-sha256; cv=none; b=sWznXvMAVyM/WkUs9RNJFjoUg3mfJBHgxNl8SbW7xpZd0KYm7JZHm0sGNKwaDn0lnhNg0Q 63Zxm8qsedXjdp6RxL7FljaMVFLuzW0A2a6DNfMYQMjJ6XXjqBnFzWaEGz7CO2Md1Nb+hg 4xPEslTqwAdXRZnwNJ+m76Lsgcbotjk= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688454409; 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=PfE66iesbIv0sKay0eFC4voF5VEy99UHsBdgYsnLi0E=; b=78R2F9rffLa+Bed6mKQfjVrgWNovQtPOK5d3EkWun+j13rKUyCj1uS4gN2lDtF0YQYByL7 kDW0ZabaAE5IuBbXGLT9iHTJVL3ej3/mUZN1PMF4LQDD0BYZLoIjuLxopuWVn2CH1xFEm6 t1PvutzceffOxZDLvGSyOOr6WZTDjIE= 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 417272F4; Tue, 4 Jul 2023 00:07:30 -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 574903F762; Tue, 4 Jul 2023 00:06:45 -0700 (PDT) Message-ID: <82d3332e-beff-85ff-c1fd-0d46345b69b5@arm.com> Date: Tue, 4 Jul 2023 08:06:42 +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] mm: riscv: fix an unsafe pte read in huge_pte_alloc() To: John Hubbard , Andrew Morton Cc: Albert Ou , Alexandre Ghiti , Andrew Jones , Hugh Dickins , Palmer Dabbelt , Paul Walmsley , Qinglin Pan , linux-riscv@lists.infradead.org, linux-mm@kvack.org, LKML , James Houghton References: <20230703190044.311730-1-jhubbard@nvidia.com> From: Ryan Roberts In-Reply-To: <20230703190044.311730-1-jhubbard@nvidia.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 3299D100017 X-Stat-Signature: xou6boynrya6pcpges1yqbwyqxyigk11 X-HE-Tag: 1688454408-499694 X-HE-Meta: U2FsdGVkX18k0AdMJnmwCbWGU415N6a1IV8MW6AJuIiBQnEDj933jTJBAfSccV9VXJOUgCEJIMGaYBeJwSMdEbVutGFXakVdzaaPtqiMcOD2Bk/DuiH60sdAJOc2NjBzOSVwseTB90hXjR4eALnMcO11cDAE7LP/qBI34bFMV4kuF/MPRuzvoYOxRHXybFIGzp3v0KieXeCO9BiDdBb90rzlAo0R1ElxZscvhthuynMuOI0qkx4FMtmQ8H/S9Sud4jQeaLoUeVHioQhPO7LzBUbfa2dvmvmrCNKuEQftsGPFf49cg380ZKdQv6ylvdJuzMjJsvgyLTs2P5ttU6RNImFRhT8j1+0VTo/eQah/SBQcDP7iiiGlgF8ql8kDVlF79ZSlUDuZn61QtcAuNJlGLps2uuUF3IpQ9xpcyAmL0/N4KXvOD3pr1Z0GCZ0tSbmYQ/aYqfQ48UKE++GFh0mvNRQcNTln6Cv3xsYPFeGC5eUazAPPfY9kEra+HhxX0896FHl1e94K9xMe4bV9XYlXIm47QizeAFpm5aTdI8NYvxr/vOye2//HS4wYDcaS8rJktbLpCmjeZlyh+3vNTtSQBYV3tVNl1lT1OQ0akN+WngZCS8mdHqDA3CdsAhct5+w2OX/eKFA/g+0/bGLUs5TCmQK4WsF6j9x9ZaHakW+0q6Bo/w+ZQsCOztskLjObx+E8387BYyDwjg9iAfRjEDSQqgRMYMGUZhbuuTq+GEDr2f5/afcdO4XroRlltPCpwMGzRtCpR8cHbC2jm2mjI8rKgwjZJE4dwHv0wX0CoDgM7p6bfewngBqOO1qY5Ak9OpmooUH+O+G+FcjDNnT0VfWoIZAtEDGshSIpAitbTf46dLAuxbEIEsCo6b8m5NOMCxImEE0+keiJxT12dfNUMYdGG5rMUhn7huoZVhTAlpGRFJvByjdda4RUhsaOmlgDKODiR9HDwpf4XWGFobuLnO3 0PmDojMT yU46IW5xE5ObF6mOJNzz62OvJzTAzshAPQynEGcLYDlJ3Oz7btORNha8tKzs0Pat00IdauseAAXa3cJtE3nEzmop4BcTcQopZCUER1atZoFxumEHpTr2S0K7Ltzguah46d+sxESFPiAf8ZOhAKhqlWnIIAsHcjH1kwdXRd00dR5EaMJIFPbghU9JKfGfQyzK5fu6Qfj7/pyLzVY04hpVJto4w1p05OioSGD9VoUm5OoGiWHgHbfL/ALMVNyUXsF/DxaCC+uYfTvUNAOXAi/crlHNa0f685Hswio+xaNuILaZAiK2qkcY8I01eRx4BssAitnY+ 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 03/07/2023 20:00, John Hubbard wrote: > The WARN_ON_ONCE() statement in riscv's huge_pte_alloc() is susceptible > to false positives, because the pte is read twice at the C language > level, locklessly, within the same conditional statement. Depending on > compiler behavior, this can lead to generated machine code that actually > reads the pte just once, or twice. Reading twice will expose the code to > changing pte values and cause incorrect behavior. > > In [1], similar code actually caused a kernel crash on 64-bit x86, when > using clang to build the kernel, but only after the conversion from *pte > reads, to ptep_get(pte). The latter uses READ_ONCE(), which forced a > double read of *pte. > > Rather than waiting for the upcoming ptep_get() conversion, just convert I'm not sure there is any upcoming ptep_get() conversion for riscv? Not from me at least - my focus was on the generic code to suficiently encapsulate it as an enabler for some follow on arm64 changes. > this part of the code now, but in a way that avoids the above problem: > take a single snapshot of the pte before using it in the WARN > conditional. > > As expected, this preparatory step does not actually change the > generated code ("make mm/hugetlbpage.s"), on riscv64, when using a gcc > 12.2 cross compiler. > > [1] https://lore.kernel.org/20230630013203.1955064-1-jhubbard@nvidia.com > > Suggested-by: James Houghton > Cc: Ryan Roberts > Signed-off-by: John Hubbard Reviewed-by: Ryan Roberts > --- > arch/riscv/mm/hugetlbpage.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c > index 542883b3b49b..96225a8533ad 100644 > --- a/arch/riscv/mm/hugetlbpage.c > +++ b/arch/riscv/mm/hugetlbpage.c > @@ -73,7 +73,11 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, > } > > out: > - WARN_ON_ONCE(pte && pte_present(*pte) && !pte_huge(*pte)); > + if (pte) { > + pte_t pteval = ptep_get_lockless(pte); > + > + WARN_ON_ONCE(pte_present(pteval) && !pte_huge(pteval)); > + } > return pte; > } > > > base-commit: 0a8d6c9c7128a93689fba384cdd7f72b0ce19abd