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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1EE8AEB64D9 for ; Tue, 4 Jul 2023 07:06:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230156AbjGDHGy (ORCPT ); Tue, 4 Jul 2023 03:06:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39574 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229546AbjGDHGu (ORCPT ); Tue, 4 Jul 2023 03:06:50 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 9324FFB for ; Tue, 4 Jul 2023 00:06:48 -0700 (PDT) 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 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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