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 6CBC3EB64D9 for ; Fri, 30 Jun 2023 01:51:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7F5FA8D0002; Thu, 29 Jun 2023 21:51:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A5C78D0001; Thu, 29 Jun 2023 21:51:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5F8928D0002; Thu, 29 Jun 2023 21:51:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 4DBC98D0001 for ; Thu, 29 Jun 2023 21:51:35 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 18009AFB8B for ; Fri, 30 Jun 2023 01:51:35 +0000 (UTC) X-FDA: 80957737350.01.C04A5B5 Received: from mail-qt1-f177.google.com (mail-qt1-f177.google.com [209.85.160.177]) by imf28.hostedemail.com (Postfix) with ESMTP id 4F978C0010 for ; Fri, 30 Jun 2023 01:51:33 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=jpctnlSk; spf=pass (imf28.hostedemail.com: domain of jthoughton@google.com designates 209.85.160.177 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688089893; 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:dkim-signature; bh=ReJelaju0aoDb9wHBJOMrRL3RwmVio6acM2bFwwczSc=; b=RGlc6rySCBSPwpgCtU7e3V7VCopU4rnbqkvqC+to3xUm46+3Wd+JAFQXPTGpfrK7YTxRtZ GE3YkKsKc6wOjGxV6fQk/AuOtxNHn3yMBfPW8jaHu333ev37EYQGzaAN/Myxitg21RR9OE C50r/SQDabUy6a9meki9NEmlyk57VlY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688089893; a=rsa-sha256; cv=none; b=Zys/IMuc/xzp7w8+KKu5R2srWpFPXeySE39oQ+XLQRW4xDfwNLB3qSeGzABwmiVPK9mBQL mgrtkkZvwDgFqInn0JtyesKLuaKASeEdldTqmy/6ESvDc71HjU01DeJP77Av0UTJryOgRY gbF/jBfGEMh3LoBtQOYyQya1sjoMCYM= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=jpctnlSk; spf=pass (imf28.hostedemail.com: domain of jthoughton@google.com designates 209.85.160.177 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-qt1-f177.google.com with SMTP id d75a77b69052e-40079620a83so146381cf.0 for ; Thu, 29 Jun 2023 18:51:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1688089892; x=1690681892; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ReJelaju0aoDb9wHBJOMrRL3RwmVio6acM2bFwwczSc=; b=jpctnlSkpJKJTWFXfWpQpjeHNbz1NDffy0XtWaM1WStlgni0XTMLdQy8U/GGYBxIVn qahrBC5Xea9x35QXceoubR6HQwpNoiZhzbekxBGcvSvag3+nM0Owo/UcHmS4qQo3pAbZ 5aqMCZdr/bxOuxybzLqqc865Y82l8ZlNcfmEDWWARFmj3LExMt5fr44hZFm+DhIilJLB Uj+dIQUu8caEr4h8TNhvG71bH70ROVIC4psg9KpYsOBFMbT1h3XQcgLvJdYlUB7OYl5c d6/+KtnUFfd+YcaW1sQPpigh3w1iucc6NPlzaa3mfeagsdvi+pI6M3jYFgnJDWRhAO1f JHsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688089892; x=1690681892; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ReJelaju0aoDb9wHBJOMrRL3RwmVio6acM2bFwwczSc=; b=M0qXP15ZObwmwBtPu1wyI88uDm+AZzPNJq8dwRQ8pmrGanZMUylh1FWWd3ZHk1XoNR uEzdxYcBtnFS5qLcJDG5k1wfoCV1GEALY+9CuH53bhSjI1zDHIebVmcmGnQcJpPuk8iz vMMGzCmRBg2tBueAsMQcKQTrsPJy7ygYDuWyWzXi12UHAeN69YvvL4yL/wNWPvXUx6Eh bcW+XOinR0EaKCMYXjKpJEy/UV530Cn+rvcQxSmrVktGq5D56eTrDn44Vtd31Jrndu/G Iy+h+tMor4nkmfsz8vDlqApRPDINmlizHVkZ1/8SfMM8zmuXeUybv1S4fySLnlCpiSPn NNpA== X-Gm-Message-State: AC+VfDwY3q/U7kys/X7DYxlJfs2bEdxdDo4vfOf16osj1Qhx2sVTz9Ia OcQ0cNoHTeNCosAGwNPxAOsiHEzCkVvcNkvI40ST/w== X-Google-Smtp-Source: ACHHUZ6hmFErQNQzWtOG6NXh3DrkAmCuI9UlIZ+Om4WUNnZW0VsV9MWGpYl2XvpPW7josv32svBXSMb6Q2N11IHLXMc= X-Received: by 2002:a05:622a:285:b0:3f8:175a:4970 with SMTP id z5-20020a05622a028500b003f8175a4970mr648134qtw.18.1688089892215; Thu, 29 Jun 2023 18:51:32 -0700 (PDT) MIME-Version: 1.0 References: <20230630013203.1955064-1-jhubbard@nvidia.com> In-Reply-To: <20230630013203.1955064-1-jhubbard@nvidia.com> From: James Houghton Date: Thu, 29 Jun 2023 21:50:55 -0400 Message-ID: Subject: Re: [PATCH] mm/hugetlb.c: fix a bug within a BUG(): inconsistent pte comparison To: John Hubbard Cc: Andrew Morton , LKML , linux-mm@kvack.org, 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 , Muchun Song , Namhyung Kim , Naoya Horiguchi , Oleksandr Tyshchenko , Pavel Tatashin , Roman Gushchin , Ryan Roberts , SeongJae Park , Shakeel Butt , Uladzislau Rezki , Vincenzo Frascino , Yu Zhao Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: jqomxmuaydqh1u71hf8pbqsicw4nir9x X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 4F978C0010 X-Rspam-User: X-HE-Tag: 1688089893-952863 X-HE-Meta: U2FsdGVkX19Z5VkL+gbykVlUcXZ/boOKzEigSUzHvOl+JLjwTFmeRn9LhlBH3wF7ifQ6firqkRLzGnIisYAyiWMweHeuo5G6TEdKB5W1PAEgbCN7dL2hdvuelCILyB8kWc5BqDanTH4QZubKcScTnPmauwGZ41vCB0gCfIqlfQYRunJJqbnuWe9cpAYi5TaUohwvlKnKkiYKtoNB2OuDTLJK1UqwUddaU5MMjdkT4kXqSkSdWteN6BWKmhf4q9SIUeaC2SVDsUc24PigArGG6+PA8fxAysRN+qZRPvYFdh2kPLhrvyUoBRYYasDGFSVwZJPmcu1nrBU2serkKCar5ys9oHbMjVVjp6ALKtwIbSQHw6T4A4DQddBJhWt0ST3tNr8ElYKYaj3CXWnPjwjCodq2Z44HgqQAK4eoECPdb5BIWE8tDCo46oEvGeVZvP50AKn/mUio0kD8+Rm38VcY/LDSFem8ZFi04wThxLG0SpT6WTMfGkx+R3rSUCgBJ4D2vTQqKaOJvdK2lkqX0iOuulv6NMFqHfuDEfR4/AVAhOopjsfVbJhXTjsMZHGAEx7I1huBdjuX0XzrwOwgVBw10vyJcn2iVmocz7Zag/BOM2LIqBzEtEgvA5eWa+PfU13dPEDI60TcozDOPSnVsf8EZ7pwITU8grMfvLaoLBavpJd4G0JCLjbdVLOlDpgxEdJXWdT9OT+eNFkSxTKKdkSser5ImQ/U8x5Wp6bxGxi0eXJxDY9l7XDJsTKE3keWME6pcva+u99gXFANFGutRkXt9diFZ3YG87Rm7temw4yKtK5fbqoQ5LeEWMsnJhyTfDFRw09J2k6m1MqIZGdyLMPsZl5ep8b10Ffiizqipfhv+GfiSegjf/nM54nlxXCWOXpqZHTgve9va0kQNf0TA5jZgpzPYoA+MSw/87DbPfeyO/RGEqiBitCgfYeQU/EnExBYfQLnQwybYt30l0IURyJ tN2RCJgK 6ibpe9dGjWt0TJhs4J/li0Zt0ZWAl33QmT0uIaIPkoRITucxRlaSXYa1YtGjVRZA+9HNCTKwAaVysmHdMtYfk2saZrLUJVamqbDum0K1+InT2V0LhXRUSD4VaAgNO7Ya4lgtN7zvw95opFYGTiKQ+t0iZAwf68B8+sMUqxARKepJqnswdk3nRHRcLeYomeeuDuiueJRjc09w/1dpc1fQeXglsCM6Jh5Q2D05dgIHlODfVFtT0RLDprTaPgVSAWkjUm5OOEHpUbstVAxbo8POlArp8j4y6XrHdUlYKT6cAmxFfzQ7mOQXleiPUEw== 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 Thu, Jun 29, 2023 at 9:32=E2=80=AFPM 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") Hi John, Good catch, and thanks for the detailed explanation. It looks like riscv and powerpc have equivalent problems in their huge_pte_alloc implementations, perhaps it's worth taking a look at those. (riscv looks like it has precisely the same problem except it's a WARN, but powerpc looks more interesting.) Either way, Acked-by: James Houghton