From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1F4DF22655E; Wed, 28 May 2025 21:56:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748469374; cv=none; b=UXNLBHE0nE/X6AWnSBvD3MAyRF/DmBzcZy2T5+Sw2j4Bwqr3zbjxd+3KWhysSIzc/X7KC8xHX/b6nqXCvcwZ3/GDFA7a3z77ff/ZUOAP2WfJa087Snk1Q+up5GOg2JizAL+vYEVbFEBMoxSeW5vpPbyQxl9GPILBGBbbEPkb7Eo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748469374; c=relaxed/simple; bh=2VThknpY4Vs8QWakS9sWYR2G/1Ip+G8eS/08RVfhu18=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version:Content-Type; b=CrP67qm60oIMhd9Gch0tdBxiVS4ijaGi3EPvaBknd/E4k7LkIbOtDVUPWziOGI01NmBrOFLrOvpVCY6sxlwXrKkaSb0Xx0yPDwKqm95b2oe61PZoTKvWHZ6ejdaQtweoTvEKsTzgo8RtJLCcwz17/7sokmAueM/689E5/hqiSkg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Sj+VDPW6; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Sj+VDPW6" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B777EC4CEE3; Wed, 28 May 2025 21:56:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1748469373; bh=2VThknpY4Vs8QWakS9sWYR2G/1Ip+G8eS/08RVfhu18=; h=From:To:Cc:Subject:Date:From; b=Sj+VDPW6AkU+hUfcg8bxTXr7+AkZMEZvknDeXWuu1nmMPY46WLDQZEW5jJ/HYplN2 ciDA+acn6TRAJ8+s84PGWaNlp1l+CC72VTiDEPfDJ4kFENbKpUOaL9jOsQm55oGg2x JZF8eokCDSKG2X1RnC/fsH8CH/igDzLJ22g8HobskNMv1GiXmpTCH7jeJA9K2u1Ni1 TQ+BvbmiR6h26N7JxakU7aSKfhaugRpazXXlf/fe5rCX8ZOwk5XMBl9VdZjIjaM6/H QW2JBcGAIqhGHAIuhcWeYAnEolsnZXkVTV3i/AsywF/kvUw/a7zl8oQgQd4aBpn8DC rqlIyGjjJzLNg== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Filipe Manana , David Sterba , Sasha Levin , clm@fb.com, josef@toxicpanda.com, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH AUTOSEL 6.14 1/8] btrfs: exit after state insertion failure at btrfs_convert_extent_bit() Date: Wed, 28 May 2025 17:56:04 -0400 Message-Id: <20250528215611.1983429-1-sashal@kernel.org> X-Mailer: git-send-email 2.39.5 Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.14.8 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Filipe Manana [ Upstream commit 3bf179e36da917c5d9bec71c714573ed1649b7c1 ] If insert_state() state failed it returns an error pointer and we call extent_io_tree_panic() which will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then we fallthrough and call cache_state() which will dereference the error pointer, resulting in an invalid memory access. So jump to the 'out' label after calling extent_io_tree_panic(), it also makes the code more clear besides dealing with the exotic scenario where CONFIG_BUG is disabled. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba Signed-off-by: Sasha Levin --- **YES** This commit should be backported to stable kernel trees. **Detailed Analysis:** **1. Bug Description and Impact:** The commit fixes a serious potential memory corruption bug in the btrfs filesystem. When `insert_state()` fails in `btrfs_convert_extent_bit()`, it returns an error pointer (ERR_PTR). The current code calls `extent_io_tree_panic()` which triggers `BUG()`, but if `CONFIG_BUG` is disabled, the code continues executing and calls `cache_state(inserted_state, cached_state)` with the error pointer, causing invalid memory access. **2. Code Changes Analysis:** The fix is minimal and surgical - just adding a single `goto out;` statement after the `extent_io_tree_panic()` call: ```c if (IS_ERR(inserted_state)) { ret = PTR_ERR(inserted_state); extent_io_tree_panic(tree, prealloc, "insert", ret); + goto out; // <-- The fix } ``` This ensures that when `CONFIG_BUG` is disabled, execution jumps to the cleanup code instead of continuing with an invalid pointer. **3. Comparison with Similar Commits:** This commit aligns with the pattern seen in "Similar Commit #2" (Status: YES), which also: - Removes reliance on `BUG_ON()` behavior - Provides graceful error handling - Has minimal risk - Fixes a potential crash/corruption scenario Similar to commit #3 and #5 (both Status: NO), this touches BUG() handling, but unlike those commits which make broader architectural changes to error handling patterns, this fix is much more contained. **4. Stable Tree Criteria Assessment:** ✅ **Fixes important bug**: Prevents potential memory corruption/crashes ✅ **Small and contained**: Single line addition ✅ **Minimal risk**: Only affects error path when insert_state() fails AND CONFIG_BUG is disabled ✅ **No new features**: Pure bug fix ✅ **No architectural changes**: Preserves existing error handling, just prevents fallthrough ✅ **Critical subsystem**: btrfs filesystem corruption prevention ✅ **Clear side effects**: No unintended consequences beyond fixing the bug **5. Risk Assessment:** - **Very Low Risk**: The change only affects an error condition that's already problematic - **Exotic scenario**: Only impacts systems with `CONFIG_BUG` disabled (uncommon but not impossible) - **No regression potential**: The change only prevents executing invalid code, doesn't change normal operation - **Well-contained**: Affects only one function in one file **6. Security Implications:** While `CONFIG_BUG` disabled is uncommon, this could potentially be exploited if an attacker can trigger the `insert_state()` failure condition, leading to memory corruption. The fix prevents this attack vector. This is a clear candidate for stable backporting - it fixes a real bug with minimal risk and follows the stable tree rules perfectly. fs/btrfs/extent-io-tree.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c index 6d08c100b01de..bb3aaf610652a 100644 --- a/fs/btrfs/extent-io-tree.c +++ b/fs/btrfs/extent-io-tree.c @@ -1456,6 +1456,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, if (IS_ERR(inserted_state)) { ret = PTR_ERR(inserted_state); extent_io_tree_panic(tree, prealloc, "insert", ret); + goto out; } cache_state(inserted_state, cached_state); if (inserted_state == prealloc) -- 2.39.5