From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yx1-f65.google.com (mail-yx1-f65.google.com [74.125.224.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DC2E0220687 for ; Tue, 2 Dec 2025 17:17:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.224.65 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764695870; cv=none; b=eUK9t0v6wpryBQmXHp4HmPDelz2/39CBaV+abtjld2bVJ/QPZ+WeB4TDjHubd5p/I8C9c4xdOgFPTL2Nlc/wI32u44tHDKAcmjrAcIrbfvp6Ms40fNSO0hJMdM5XlIHoXbBcuVZ0KHMX2EVTgwiL5RjRfhlHA+46g74B9kuGxfU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764695870; c=relaxed/simple; bh=e37SN+aToRVyC6n1zkZmulujPisP2nXDQSMcfkqxLY8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=CsA0c8Ud9xC+kgAyFjN0ly6ZcF0I4vVuIemeoNMfdNofkPnjDy1HFnf5fhqLiwL/TnCVAgpSN/HUZkJnI6MgKRYC9Yro7KdQmJl7QB/4B6heJg2O7tZ4BsfXaxeBkrArBhqzG/BrCicgNIK57mkkyA62lDuYE3t0msaoPzMkwBQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=bSaONty5; arc=none smtp.client-ip=74.125.224.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="bSaONty5" Received: by mail-yx1-f65.google.com with SMTP id 956f58d0204a3-640d790d444so4851721d50.0 for ; Tue, 02 Dec 2025 09:17:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764695868; x=1765300668; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=Q8QlVTXiL4fK7xIfQtCmFRQsL7zA4OuWCxdMWpV4kkE=; b=bSaONty5aT5muOIMYOcyUQF+6kKOrktNRPQ4XXJ/ltkyxtio87plPSRlGH34E61mMD r38Rlv3J/ciK9lWksDvQyg+sZS4rGVVjiFj7Y9wIowzWwqQTrsqzXG/XlS/Oh+Cr0qZ/ oVcFbA8GThn5uxPg5CrW+vMqLF0XCqFJ1SPN9eQnN6547bRtg/Qo2XWsqjFDTGeLvyYU HXG7ogcluGmCg6H6r8urWViuzja4N6gsCA+S8spUYMZ5mH/05GKNaB5vPFTGwtB4H+6Q JVNvmn1r+YMbeuNxEvAx7Pqx53D0s5WUq2dsFm8Y8D2g0j77nTm/WgAiwHxaq7qVMtxB 2w8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764695868; x=1765300668; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=Q8QlVTXiL4fK7xIfQtCmFRQsL7zA4OuWCxdMWpV4kkE=; b=qKOgTbAZzxCzjnKMDXL/977W84Na0AnOniAOOGehApiMk9e0/AowT96caH/KWFYnnP 2rltP3zWnqqDsJ1FQA2nwXtr0dj5k6eBIr+Ol6jfcp23LlVgZjbVgKahy/MkbX/OdP0p nYNAPrN/iiagCh+SYOQxqSUkWn5dBs0W4oAGUzMZRXx1Uk6dLHln9wMar7tYM5hFBmvQ yJZE/rgIlGPtCJ8fZuqE4kV7kOJWpYUndnnEv1PLj9Ikb7thG11qLfo3phxaatDtSNiV nA4T/hsGcdgq874fZZwH7LAWmjmL+1wZF2UC4FY9Q5JxRclujUv0JsnFrnlriR6mMIVk FTsQ== X-Forwarded-Encrypted: i=1; AJvYcCW/5U0WitpBhklFg5waYmKMQfndd5RK015ZxGNz6XOgmr5AkJm79ORArarNHnY1/CiAkFnyNp0spmpeTlQ=@vger.kernel.org X-Gm-Message-State: AOJu0YwEcVv1ABWhDXdZc8sSA/Z+v5J3yasPfWVbpjGtAZvNWwUjGHna WzsGhgjsHn3kvXrQYYPq1nm6MXzT0bl8/KWNIREP8gh3CfnhrMTRWOAl X-Gm-Gg: ASbGncuFyr+0JpoPhgjV74cmo5Q6X+rjIqJ4M9SzFWNKWm58ZEV6Z90Da8rmvFItpGw BUoW6CAYCz7vNWj8tVF6/GHw9DsBRzTM4JWWNIgiv25Nf8hU3HRUPu/5uX5KF6zr5snAzDl5y/5 uc/D3vI8aRmdSxyB0dLpCb0+Sv2H8iGz9RyQ3h2ztWcLKNa6uOfP1yzTlcDTyvPE/lOtxbizpIL pu6QtIs3hM6+grh+1iC0Pi1N7sIXJe3rEuVPZLEKLPfKpLSagyxBwzfPl1qaBEl1yy+KSN7LGSR +ES958Jk73lMUPjE9DUACdv+h4ElB/UdCLMgFDLmO17Gye/R7Yo7FcrcQDXA95qlJFhcT5/6cT2 18P9pZBYm2+127HFbsG9EwL15vRbHFVrjm5WyLvV89xwLFOwoGgJcCFN9E8xHn3iMfQT8cfXe00 meBnimN03OJBDoHVwh X-Google-Smtp-Source: AGHT+IHdKu4hpE/8f2eE+4lPs0EO9wbGhycCa+sCjZHw1sF1sCEgg+Iae1BCxPfWRTEdAx3ecTHilw== X-Received: by 2002:a53:c052:0:20b0:640:d0d4:526d with SMTP id 956f58d0204a3-64302a2dc80mr25410432d50.9.1764695867800; Tue, 02 Dec 2025 09:17:47 -0800 (PST) Received: from localhost ([2a03:2880:25ff:1::]) by smtp.gmail.com with ESMTPSA id 00721157ae682-78ad0d82295sm64124477b3.25.2025.12.02.09.17.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Dec 2025 09:17:47 -0800 (PST) From: Leo Martins To: Filipe Manana Cc: Oliver Sang , oe-lkp@lists.linux.dev, lkp@intel.com, linux-kernel@vger.kernel.org, David Sterba , linux-btrfs@vger.kernel.org Subject: Re: [linus:master] [btrfs] e8513c012d: addition_on#;use-after-free Date: Tue, 2 Dec 2025 09:17:40 -0800 Message-ID: <20251202171745.798946-1-loemra.dev@gmail.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Tue, 2 Dec 2025 15:04:51 +0000 Filipe Manana wrote: > On Tue, Dec 2, 2025 at 8:40 AM Oliver Sang wrote: > > > > hi, Leo Martins, > > > > On Mon, Dec 01, 2025 at 04:51:41PM -0800, Leo Martins wrote: > > > > [...] > > > > > > > > Hello, > > > > > > I believe I have identified the root cause of the warning. > > > However, I'm having some troubles running the reproducer as I > > > haven't setup lkp-tests yet. Could you test the patch below > > > against your reproducer to see if it fixes the issue? > > > > we confirmed your patch fixed the issues we reported in origial report. thanks! > > > > Tested-by: kernel test robot > > > > > > > > ---8<--- > > > > > > [PATCH] btrfs: fix use-after-free in btrfs_get_or_create_delayed_node > > > > > > Previously, btrfs_get_or_create_delayed_node sets the delayed_node's > > > refcount before acquiring the root->delayed_nodes lock. > > > Commit e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes") > > > moves refcount_set inside the critical section which means > > > there is no longer a memory barrier between setting the refcount and > > > setting btrfs_inode->delayed_node = node. > > > > > > This allows btrfs_get_or_create_delayed_node to set > > > btrfs_inode->delayed_node before setting the refcount. > > > A different thread is then able to read and increase the refcount > > > of btrfs_inode->delayed_node leading to a refcounting bug and > > > a use-after-free warning. > > > > > > The fix is to move refcount_set back to where it was to take > > > advantage of the implicit memory barrier provided by lock > > > acquisition. > > > > > > Fixes: e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes") > > > Reported-by: kernel test robot > > > Closes: https://lore.kernel.org/oe-lkp/202511262228.6dda231e-lkp@intel.com > > > Signed-off-by: Leo Martins > > > --- > > > fs/btrfs/delayed-inode.c | 34 ++++++++++++++++++---------------- > > > 1 file changed, 18 insertions(+), 16 deletions(-) > > > > > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c > > > index 364814642a91..f61f10000e33 100644 > > > --- a/fs/btrfs/delayed-inode.c > > > +++ b/fs/btrfs/delayed-inode.c > > > @@ -152,37 +152,39 @@ static struct btrfs_delayed_node *btrfs_get_or_create_delayed_node( > > > return ERR_PTR(-ENOMEM); > > > btrfs_init_delayed_node(node, root, ino); > > > > > > + /* Cached in the inode and can be accessed. */ > > > + refcount_set(&node->refs, 2); > > > + btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC); > > > + btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC); > > > + > > > /* Allocate and reserve the slot, from now it can return a NULL from xa_load(). */ > > > ret = xa_reserve(&root->delayed_nodes, ino, GFP_NOFS); > > > - if (ret == -ENOMEM) { > > > - btrfs_delayed_node_ref_tracker_dir_exit(node); > > > - kmem_cache_free(delayed_node_cache, node); > > > - return ERR_PTR(-ENOMEM); > > > - } > > > + if (ret == -ENOMEM) > > > + goto cleanup; > > > + > > > xa_lock(&root->delayed_nodes); > > > ptr = xa_load(&root->delayed_nodes, ino); > > > if (ptr) { > > > /* Somebody inserted it, go back and read it. */ > > > xa_unlock(&root->delayed_nodes); > > > - btrfs_delayed_node_ref_tracker_dir_exit(node); > > > - kmem_cache_free(delayed_node_cache, node); > > > - node = NULL; > > > - goto again; > > > + goto cleanup; > > > } > > > ptr = __xa_store(&root->delayed_nodes, ino, node, GFP_ATOMIC); > > > ASSERT(xa_err(ptr) != -EINVAL); > > > ASSERT(xa_err(ptr) != -ENOMEM); > > > ASSERT(ptr == NULL); > > > - > > > - /* Cached in the inode and can be accessed. */ > > > - refcount_set(&node->refs, 2); > > > - btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC); > > > - btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC); > > > - > > > - btrfs_inode->delayed_node = node; > > > + WRITE_ONCE(btrfs_inode->delayed_node, node); > > Why the WRITE_ONCE() change? Since there are lockless readers of btrfs_inode->delayed_node all writers should be marked with WRITE_ONCE to force the compiler to store atomically. > > Can you explain in the changelog why it's being introduced? > This seems unrelated and it was not there before the commit mentioned > in the Fixes tag. I'll send out a v2 without the WRITE_ONCE since it is not directly related to this bug and send out a separate patch updating writes to use WRITE_ONCE. Thanks. > > Thanks. > > > > xa_unlock(&root->delayed_nodes); > > > > > > return node; > > > +cleanup: > > > + btrfs_delayed_node_ref_tracker_free(node, tracker); > > > + btrfs_delayed_node_ref_tracker_free(node, &node->inode_cache_tracker); > > > + btrfs_delayed_node_ref_tracker_dir_exit(node); > > > + kmem_cache_free(delayed_node_cache, node); > > > + if (ret) > > > + return ERR_PTR(ret); > > > + goto again; > > > } > > > > > > /* > > > -- > > > 2.47.3 > >