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 D98FBEB64D9 for ; Tue, 4 Jul 2023 06:01:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 393BE28005D; Tue, 4 Jul 2023 02:01:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 34410280049; Tue, 4 Jul 2023 02:01:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 20C1C28005D; Tue, 4 Jul 2023 02:01:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 10D8F280049 for ; Tue, 4 Jul 2023 02:01:12 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id C80FFB0265 for ; Tue, 4 Jul 2023 06:01:11 +0000 (UTC) X-FDA: 80972881542.18.F3DFF62 Received: from mail-ej1-f42.google.com (mail-ej1-f42.google.com [209.85.218.42]) by imf27.hostedemail.com (Postfix) with ESMTP id DBDB34000E for ; Tue, 4 Jul 2023 06:01:09 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=ventanamicro.com header.s=google header.b=C+63iSwA; dmarc=none; spf=pass (imf27.hostedemail.com: domain of ajones@ventanamicro.com designates 209.85.218.42 as permitted sender) smtp.mailfrom=ajones@ventanamicro.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688450470; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Nl9gTJEQlp1epsps/bWt1kzzLQTu7OGXe4NvbD2+dJU=; b=EWAWxaltYcq/3EoOleF4xdw9kHBLLby5v5pfC26x88rj539mOBvS0Tq1VsR9au2k0Ajut4 fw8eCtHokzGOQRvIZMWcslKxcMqXwtWxwTKU3YNHY5/jnNNPw8HN47IS4LG/Q4EWNjrvjm WO/nuBCGSkADQmdloi1UdIRyWv6cKCE= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=ventanamicro.com header.s=google header.b=C+63iSwA; dmarc=none; spf=pass (imf27.hostedemail.com: domain of ajones@ventanamicro.com designates 209.85.218.42 as permitted sender) smtp.mailfrom=ajones@ventanamicro.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688450470; a=rsa-sha256; cv=none; b=4r+3QulScQAlGI0Xvgl+RSNeFUStK57nihZovU0Xhh+G+gNMblQr56bS1zFLW+qcoaHDRY mE5cY7NKwZEGmy8LUfIQyoeFKbCrD/578q7mc7/Ligk4/OS458ZSmyftcpN9+AV5YWFjry qbYFRDDGtbgcTw8e9LpGN/WnyPsbVw4= Received: by mail-ej1-f42.google.com with SMTP id a640c23a62f3a-986d8332f50so613203266b.0 for ; Mon, 03 Jul 2023 23:01:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1688450468; x=1691042468; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Nl9gTJEQlp1epsps/bWt1kzzLQTu7OGXe4NvbD2+dJU=; b=C+63iSwAWL2QEv0vXOTtpkTpOzgfgDV8ArF5AQ98M1x0FpUFDjwYxEkTSpOLpEZDTz lv5BozU1lxxx9H6vVvEOm8YKdTyb/wg/78/5TRW+J24ix/xuhXIZwSkkoDmbiot/4cr0 QCyIyGYVokfAlzaHFR6DwmaVJRRSCKFe6BGuca/G8BawV01HkxYVpjJUSkVrk0c2mtD9 tln2SulLzmzn0G3Psk4yJSliN82FGCwckLBBrKrthFxAkln4Zj9cLUX78weHlfkzIY8I SI/HaWTYJrNHfzbs2EE/jhilLXaCsjxKdB3Cg1syg7rz/aBBQiX/BI+dn9npt/2cmNQD 8GxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688450468; x=1691042468; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Nl9gTJEQlp1epsps/bWt1kzzLQTu7OGXe4NvbD2+dJU=; b=CiP4L0jrvUzHuoyJckx2vhJHa7pDZx7EDFm7zk8E0DmSXsvr33oyzjyEV6MkY8th0L CJ4PYPT7FfCRtpIzMxBYG3+Vr5mu9zXWN8mWAYiYcxMEcEBolXV4Wr4ieLox0cFLW87W RRPsHjCQBDwbw8ABlLAk0jdFEPA3+zRpWZ1EWoIW0GjWyT/jZcEBlgPjP1LMV5ctTFiA rjuk56LwJ3lK60bROEnprvw9ErJ4AeilEEz6iWgv4CuVhJg+R8QflGNPkxZVS8b59X9J /3yW3KK3/X61yVsLhURNkl5QutTCPDgJ63+dpHqnnAMCM3GnYG4HkwyOgrjxH6JVvgcf i9cQ== X-Gm-Message-State: ABy/qLazyVZ9ZFWNWzO6CP1M5x9I3sOb37PTu1vaSWyAmre7bS704MG1 3GaezPCB9lOnlnHJ4nPAMb/aXw== X-Google-Smtp-Source: APBJJlHQg2PkEmmcSnTVZtihp1VqBSr2PqMhFlzgoDfMBKwN+COmCPeIF8RS9L6pzXrtBhJgf77Q/w== X-Received: by 2002:a17:906:74e:b0:992:3aa8:b21 with SMTP id z14-20020a170906074e00b009923aa80b21mr8972282ejb.25.1688450468186; Mon, 03 Jul 2023 23:01:08 -0700 (PDT) Received: from localhost (cst2-173-16.cust.vodafone.cz. [31.30.173.16]) by smtp.gmail.com with ESMTPSA id i26-20020a1709063c5a00b00991d54db2acsm10381224ejg.44.2023.07.03.23.01.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Jul 2023 23:01:07 -0700 (PDT) Date: Tue, 4 Jul 2023 08:01:06 +0200 From: Andrew Jones To: John Hubbard Cc: Andrew Morton , Albert Ou , Alexandre Ghiti , Hugh Dickins , Palmer Dabbelt , Paul Walmsley , Qinglin Pan , linux-riscv@lists.infradead.org, linux-mm@kvack.org, LKML , James Houghton , Ryan Roberts Subject: Re: [PATCH] mm: riscv: fix an unsafe pte read in huge_pte_alloc() Message-ID: <20230704-f273e5ba6c440dff03d07101@orel> References: <20230703190044.311730-1-jhubbard@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230703190044.311730-1-jhubbard@nvidia.com> X-Rspamd-Queue-Id: DBDB34000E X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: 1m5d9jadcs84qxg8wmqzidfugiwwi51i X-HE-Tag: 1688450469-605325 X-HE-Meta: U2FsdGVkX1/JokhYOI8Gl1ZgBLUac7H6SQxPho5cAT7aa/kAHlU2HCKzaK4feEuD8I8HJbQr1THHMPn9lrV5uQ9j9+rzUk9fIC+mq+baOEDUmwo0Q/syse/sfxcewCQxLCuGlh0QI1WC0IQQZbrcvfmUyLcQ+pSBZOz2Gx3vzVXtUVeKNZXWwnjwb6z9O+lNvCnID3m3eJrsUSLcbLGUvOGrJ9+lNwggzY8JGUUJsPXMojHr8gUaZh1IhYZAtmSnFYjW+wPdrKuy/DD4gdPyWCnD3niMue2VX70kpUhrMG1Y59+x53IGE29MYNVRE+hhuWH0K9/D/M9hmWMCF17+k3mQPmpJn/l4DyAbhh6tVJuepprcTvVGr9XjDTX2s11yFGi8sjomdgLChGVdg7X5AiPLIefEmvcZLYJwHj6zzo/V3U/yy/NU1iz7blEcaerJQ9isXBNcegrDro3iUakxEIAP6y/cqb46OJPV9loIh3VA6HAna/whYUWZKt0TndvSJ752+1Hx7P6J+U9wpsAGZceNpqXa0MYSZFtHFsvuJ9cJWdkaOGqM0Xyqkkm8JO5B8sJk/W+BLK0ahx1FmXsuYxQ5jB8h2yxlB7KzoefxMUWEpb4jpNtvhaFB6aIWKfPf72hDl9HGug8tR3zcZesvptXXJgI3pi61LJa4aip/WZHnWPmap6CWSEC+M9VFManpZBsscoxnaSWRyoL5czU7q6Dw1hlQQpdZSKo0hM6XyQu9+Qcujolkl1v6jX2eztThcIuNgysJpy4+2hkdVvkZlpbmXQIjfVzpiQGDhp8rXqKwQVrsVLUWDaVG2s7AI5rEQqUpOTd7tWEei/0W0K9/dRyhx5HlJCMe+DRQ04ADIFzfGuFHEhq5fh0EOQZXNQPgXPE+UJkL2DljCRVdSXA5uZygbA8/7Tdvml01RF2bVS/DtwbQLbGhezrIp2sDUhIVPo2WYJRwJtvgthJ+rni RiqYwrw9 ryXcZxQkWyOKiIwvBX8Ibr0X1xuz7dP3IckcxKJU5t+SgQN7wkX5PauoOuz2DRz7wARTHIzyTvs4llmI/vIPoV0nrXFAJ3wwcO5hHS8iK+Dv9Kj3bBgMbhiHUODJrbY44WihmWtqM1B/8T5YcbRrk/oLLMgCw77oH3hyh7N6uviUh1FORy18e4m28YeRf3atGBT+0+HtwhNraSGcB4UwiTQkP6M+13sYQb542iA9AEvnQinKs1T0e1U+V2pISfKd3LeOE+iGDLi11BTDXxkyjT687t2ycjO3r2WKKRM2o8dhGoOYat45JYQuY8P9kYkXuSdNA82vQjPvox2AuE9CzxHHVZ3m90BhKrl0M054xTG7AEwLVa/0oVFOkTQFRaBc5Ey8600I9pZun7RSu8IA77jEf9cmsvSE6iI81ufl3W2GIloXPwGQ85u2YeGpHE25NZplk2EryhSmyG5OJKkn+jdMlHnAdTlmamOxcHJibd0DSQFK+qQT1xq2oKs2WI78n2AwCDk5LcFNDTo3NMXflXdKIMfTe+nr3cbJjysjI1/kddJ848yqgVTwdmXdjsyTqJMUoevvGGsuM9PKub3qoWwzx2/u39J0KyVpx1nBDiny5ba42DS2Q/6odlQ== 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 Mon, Jul 03, 2023 at 12:00:44PM -0700, 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 > 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 > --- > 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); I think ptep_get_lockless() on riscv (even riscv32) will always just be ptep_get(), since pte_t is unsigned long, which can be read atomically. > + > + WARN_ON_ONCE(pte_present(pteval) && !pte_huge(pteval)); Ensuring we only read the pte once is good though. Reviewed-by: Andrew Jones Thanks, drew > + } > return pte; > } > > > base-commit: 0a8d6c9c7128a93689fba384cdd7f72b0ce19abd > -- > 2.41.0 >