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 X-Spam-Level: X-Spam-Status: No, score=-7.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3DBC5C433EF for ; Fri, 24 Sep 2021 11:09:47 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CA6BC60E90 for ; Fri, 24 Sep 2021 11:09:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CA6BC60E90 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 4E1506B006C; Fri, 24 Sep 2021 07:09:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 469966B0071; Fri, 24 Sep 2021 07:09:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2E37B900002; Fri, 24 Sep 2021 07:09:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0196.hostedemail.com [216.40.44.196]) by kanga.kvack.org (Postfix) with ESMTP id 19CBA6B006C for ; Fri, 24 Sep 2021 07:09:46 -0400 (EDT) Received: from smtpin31.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id BF79A2D3BB for ; Fri, 24 Sep 2021 11:09:45 +0000 (UTC) X-FDA: 78622196730.31.4077917 Received: from mail-lf1-f42.google.com (mail-lf1-f42.google.com [209.85.167.42]) by imf09.hostedemail.com (Postfix) with ESMTP id 7DB063000104 for ; Fri, 24 Sep 2021 11:09:45 +0000 (UTC) Received: by mail-lf1-f42.google.com with SMTP id b20so39076697lfv.3 for ; Fri, 24 Sep 2021 04:09:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=y2+HfOHnV0xb6zV4dOGhoGeFOUOY+WWDxbpfAlUhn2I=; b=fm2WPVBjJ0vo1CI+4Fm71l/8GvluOEGtLYt9VaS7soR1AWvxbTtxcMz106A7W/OUFW 9msuULELDYP10jKK1UT4ZbYnvHDFy7W0JuBytxCJ9Cc8ZC74gKtCbDsVM/OLqZF1hXpE pf5j2JKnwT8KpLQ6qyP2LcL1yQ0vxjePpzbDGLTbxawpwLLI77lhJ5bQzBYfyEzxt5qu +hGpxr/DLZRWTuZqHB3n4Ma3J70PBS92R8vI6a7+cGoNuK4hucJ42ML9xMescxBgZ/Pp Sd4BP6YtXCWMalf+mr+erpoMlYrNVLpWs05CyCQmTjE6krYrUw3ZMmni7T67vO42Qi2G uDfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=y2+HfOHnV0xb6zV4dOGhoGeFOUOY+WWDxbpfAlUhn2I=; b=I/Ye9M46jbwxiIVbEqO535qAOYm9DXnnxZcsaagcc8bgx9wM4PjuTxbXP7SY7upewT nHYi3IKmgoKbjG55vCg/ILMwIE5OqgZBbhg6zNsrnOarV2PGhFLPDsYbirAdZkKfEzko GlQ1GxyyftVcvmiRJuUIXKIiv2/Gr42rjJGM8lGIdHaO0wmztVQkeEaIY4/jSyqCwrSi vsuMbVFO1ANBbe4PaVZA2nvjiQNzAChd1rc6jFQqzDQTGmjNMJ+99AVX35NlfTj3lqTE UDKuH0ekjJgOB6ZmDF7RxI8WacREq3F7OhwcP5iwpIWzNaPfsMj6Tko6uZAT6ZT+zf3l f2tg== X-Gm-Message-State: AOAM531RgpKG+bpHfMqpnNAPuAbLpGqQndp1qGOSvcSmgcl4rbJZc2RQ sHOclkfbUHKYMP8OfjqRJN350uUsQxPv7zqwiPc= X-Google-Smtp-Source: ABdhPJyczi1wcMH6JE93aJAc4Y8+1lMwl194iHWW19om/BU5+/jYkPj1v46FYx5k+sNHe8qnJicXOWTM/luPEAL5iIM= X-Received: by 2002:a2e:8e89:: with SMTP id z9mr10672076ljk.350.1632481783766; Fri, 24 Sep 2021 04:09:43 -0700 (PDT) MIME-Version: 1.0 References: <20210907195226.14b1d22a07c085b22968b933@linux-foundation.org> <20210908030026.2dLZCmkE4%akpm@linux-foundation.org> <20210924103533.GA22717@duo.ucw.cz> In-Reply-To: <20210924103533.GA22717@duo.ucw.cz> From: Ryusuke Konishi Date: Fri, 24 Sep 2021 20:09:31 +0900 Message-ID: Subject: Re: [patch 136/147] nilfs2: use refcount_dec_and_lock() to fix potential UAF To: Pavel Machek Cc: LKML , stable@vger.kernel.org, Andrew Morton , linux-mm@kvack.org, mm-commits@vger.kernel.org, Zhen Lei , Linus Torvalds Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 7DB063000104 X-Stat-Signature: af8nchif81tigtaffe74g46dh9f9udi7 Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=fm2WPVBj; spf=pass (imf09.hostedemail.com: domain of konishi.ryusuke@gmail.com designates 209.85.167.42 as permitted sender) smtp.mailfrom=konishi.ryusuke@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1632481785-994863 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: Hi, On Fri, Sep 24, 2021 at 7:35 PM Pavel Machek wrote: > > Hi! > > > From: Zhen Lei > > Subject: nilfs2: use refcount_dec_and_lock() to fix potential UAF > > > > When the refcount is decreased to 0, the resource reclamation branch is > > entered. Before CPU0 reaches the race point (1), CPU1 may obtain the > > spinlock and traverse the rbtree to find 'root', see nilfs_lookup_root(). > > Although CPU1 will call refcount_inc() to increase the refcount, it is > > obviously too late. CPU0 will release 'root' directly, CPU1 then accesses > > 'root' and triggers UAF. > > > > Use refcount_dec_and_lock() to ensure that both the operations of decrease > > refcount to 0 and link deletion are lock protected eliminates this risk. > > > > CPU0 CPU1 > > nilfs_put_root(): > > <-------- (1) > > spin_lock(&nilfs->ns_cptree_lock); > > rb_erase(&root->rb_node, &nilfs->ns_cptree); > > spin_unlock(&nilfs->ns_cptree_lock); > > > > kfree(root); > > <-------- use-after-free > > > There is no reproduction program, and the above is only theoretical > > analysis. > > Ok, so we have a theoretical bug, and fix already on its way to > stable. But ... is it correct? > > > +++ a/fs/nilfs2/the_nilfs.c > > @@ -792,14 +792,13 @@ nilfs_find_or_create_root(struct the_nil > > > > void nilfs_put_root(struct nilfs_root *root) > > { > > - if (refcount_dec_and_test(&root->count)) { > > - struct the_nilfs *nilfs = root->nilfs; > > + struct the_nilfs *nilfs = root->nilfs; > > > > - nilfs_sysfs_delete_snapshot_group(root); > > - > > - spin_lock(&nilfs->ns_cptree_lock); > > + if (refcount_dec_and_lock(&root->count, &nilfs->ns_cptree_lock)) { > > rb_erase(&root->rb_node, &nilfs->ns_cptree); > > spin_unlock(&nilfs->ns_cptree_lock); > > + > > + nilfs_sysfs_delete_snapshot_group(root); > > iput(root->ifile); > > > > kfree(root); > > spin_lock() is deleted, but spin_unlock() is not affected. This means > unbalanced locking, right? It's okay. spin_lock() is integrated into refcount_dec_and_lock(), which was originally refcount_dec_and_test(). Thanks, Ryusuke Konishi > > Best regards, > Pavel > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >