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 39799C001DF for ; Fri, 30 Jun 2023 02:14:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232097AbjF3COW (ORCPT ); Thu, 29 Jun 2023 22:14:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54090 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229576AbjF3COT (ORCPT ); Thu, 29 Jun 2023 22:14:19 -0400 Received: from out-47.mta0.migadu.com (out-47.mta0.migadu.com [91.218.175.47]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C0A9B30EC for ; Thu, 29 Jun 2023 19:14:17 -0700 (PDT) Content-Type: text/plain; charset=us-ascii DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1688091255; h=from:from: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=c+Q6kGCsNOKoZaGl4/+CoNXY45hAC8Rmehd0ODubecA=; b=nBw5cd0UmlLTPBcRMaXJgc7YE0SMPKtdhLmuO73l3i6lmYUhxiOT/De7PvW9OIa2qAxDHg mA10/Pb7AmwhwRqeiQSmdiToCh59DsruaJ1F+7UnJqGswYO1K5cH8Ibxcf06gt+QMAuVpo iVNAWO95ztLaVLBd0bSJGpTZzQGqt8M= MIME-Version: 1.0 Subject: Re: [PATCH] mm/hugetlb.c: fix a bug within a BUG(): inconsistent pte comparison X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song In-Reply-To: <20230630013203.1955064-1-jhubbard@nvidia.com> Date: Fri, 30 Jun 2023 10:13:31 +0800 Cc: Andrew Morton , LKML , Linux Memory Management List , 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 , Ryan Roberts , SeongJae Park , Shakeel Butt , Uladzislau Rezki , Vincenzo Frascino , Yu Zhao Content-Transfer-Encoding: 7bit Message-Id: <070B191B-1D20-4E19-A9AB-961852FEC333@linux.dev> References: <20230630013203.1955064-1-jhubbard@nvidia.com> To: John Hubbard X-Migadu-Flow: FLOW_OUT Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Jun 30, 2023, at 09:32, 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") > 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: Muchun Song > 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 Acked-by: Muchun Song Thanks.