All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, Filipe Manana <fdmanana@suse.com>,
	David Sterba <dsterba@suse.com>, Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.10 26/79] btrfs: fix processing of delayed tree block refs during backref walking
Date: Thu, 27 Oct 2022 18:55:36 +0200	[thread overview]
Message-ID: <20221027165055.277945148@linuxfoundation.org> (raw)
In-Reply-To: <20221027165054.270676357@linuxfoundation.org>

From: Filipe Manana <fdmanana@suse.com>

[ Upstream commit 943553ef9b51db303ab2b955c1025261abfdf6fb ]

During backref walking, when processing a delayed reference with a type of
BTRFS_TREE_BLOCK_REF_KEY, we have two bugs there:

1) We are accessing the delayed references extent_op, and its key, without
   the protection of the delayed ref head's lock;

2) If there's no extent op for the delayed ref head, we end up with an
   uninitialized key in the stack, variable 'tmp_op_key', and then pass
   it to add_indirect_ref(), which adds the reference to the indirect
   refs rb tree.

   This is wrong, because indirect references should have a NULL key
   when we don't have access to the key, and in that case they should be
   added to the indirect_missing_keys rb tree and not to the indirect rb
   tree.

   This means that if have BTRFS_TREE_BLOCK_REF_KEY delayed ref resulting
   from freeing an extent buffer, therefore with a count of -1, it will
   not cancel out the corresponding reference we have in the extent tree
   (with a count of 1), since both references end up in different rb
   trees.

   When using fiemap, where we often need to check if extents are shared
   through shared subtrees resulting from snapshots, it means we can
   incorrectly report an extent as shared when it's no longer shared.
   However this is temporary because after the transaction is committed
   the extent is no longer reported as shared, as running the delayed
   reference results in deleting the tree block reference from the extent
   tree.

   Outside the fiemap context, the result is unpredictable, as the key was
   not initialized but it's used when navigating the rb trees to insert
   and search for references (prelim_ref_compare()), and we expect all
   references in the indirect rb tree to have valid keys.

The following reproducer triggers the second bug:

   $ cat test.sh
   #!/bin/bash

   DEV=/dev/sdj
   MNT=/mnt/sdj

   mkfs.btrfs -f $DEV
   mount -o compress $DEV $MNT

   # With a compressed 128M file we get a tree height of 2 (level 1 root).
   xfs_io -f -c "pwrite -b 1M 0 128M" $MNT/foo

   btrfs subvolume snapshot $MNT $MNT/snap

   # Fiemap should output 0x2008 in the flags column.
   # 0x2000 means shared extent
   # 0x8 means encoded extent (because it's compressed)
   echo
   echo "fiemap after snapshot, range [120M, 120M + 128K):"
   xfs_io -c "fiemap -v 120M 128K" $MNT/foo
   echo

   # Overwrite one extent and fsync to flush delalloc and COW a new path
   # in the snapshot's tree.
   #
   # After this we have a BTRFS_DROP_DELAYED_REF delayed ref of type
   # BTRFS_TREE_BLOCK_REF_KEY with a count of -1 for every COWed extent
   # buffer in the path.
   #
   # In the extent tree we have inline references of type
   # BTRFS_TREE_BLOCK_REF_KEY, with a count of 1, for the same extent
   # buffers, so they should cancel each other, and the extent buffers in
   # the fs tree should no longer be considered as shared.
   #
   echo "Overwriting file range [120M, 120M + 128K)..."
   xfs_io -c "pwrite -b 128K 120M 128K" $MNT/snap/foo
   xfs_io -c "fsync" $MNT/snap/foo

   # Fiemap should output 0x8 in the flags column. The extent in the range
   # [120M, 120M + 128K) is no longer shared, it's now exclusive to the fs
   # tree.
   echo
   echo "fiemap after overwrite range [120M, 120M + 128K):"
   xfs_io -c "fiemap -v 120M 128K" $MNT/foo
   echo

   umount $MNT

Running it before this patch:

   $ ./test.sh
   (...)
   wrote 134217728/134217728 bytes at offset 0
   128 MiB, 128 ops; 0.1152 sec (1.085 GiB/sec and 1110.5809 ops/sec)
   Create a snapshot of '/mnt/sdj' in '/mnt/sdj/snap'

   fiemap after snapshot, range [120M, 120M + 128K):
   /mnt/sdj/foo:
    EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
      0: [245760..246015]: 34304..34559       256 0x2008

   Overwriting file range [120M, 120M + 128K)...
   wrote 131072/131072 bytes at offset 125829120
   128 KiB, 1 ops; 0.0001 sec (683.060 MiB/sec and 5464.4809 ops/sec)

   fiemap after overwrite range [120M, 120M + 128K):
   /mnt/sdj/foo:
    EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
      0: [245760..246015]: 34304..34559       256 0x2008

The extent in the range [120M, 120M + 128K) is still reported as shared
(0x2000 bit set) after overwriting that range and flushing delalloc, which
is not correct - an entire path was COWed in the snapshot's tree and the
extent is now only referenced by the original fs tree.

Running it after this patch:

   $ ./test.sh
   (...)
   wrote 134217728/134217728 bytes at offset 0
   128 MiB, 128 ops; 0.1198 sec (1.043 GiB/sec and 1068.2067 ops/sec)
   Create a snapshot of '/mnt/sdj' in '/mnt/sdj/snap'

   fiemap after snapshot, range [120M, 120M + 128K):
   /mnt/sdj/foo:
    EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
      0: [245760..246015]: 34304..34559       256 0x2008

   Overwriting file range [120M, 120M + 128K)...
   wrote 131072/131072 bytes at offset 125829120
   128 KiB, 1 ops; 0.0001 sec (694.444 MiB/sec and 5555.5556 ops/sec)

   fiemap after overwrite range [120M, 120M + 128K):
   /mnt/sdj/foo:
    EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
      0: [245760..246015]: 34304..34559       256   0x8

Now the extent is not reported as shared anymore.

So fix this by passing a NULL key pointer to add_indirect_ref() when
processing a delayed reference for a tree block if there's no extent op
for our delayed ref head with a defined key. Also access the extent op
only after locking the delayed ref head's lock.

The reproducer will be converted later to a test case for fstests.

Fixes: 86d5f994425252 ("btrfs: convert prelimary reference tracking to use rbtrees")
Fixes: a6dbceafb915e8 ("btrfs: Remove unused op_key var from add_delayed_refs")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/btrfs/backref.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 7e8fac12f3f8..92cb16c0e5ee 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -818,16 +818,11 @@ static int add_delayed_refs(const struct btrfs_fs_info *fs_info,
 			    struct preftrees *preftrees, struct share_check *sc)
 {
 	struct btrfs_delayed_ref_node *node;
-	struct btrfs_delayed_extent_op *extent_op = head->extent_op;
 	struct btrfs_key key;
-	struct btrfs_key tmp_op_key;
 	struct rb_node *n;
 	int count;
 	int ret = 0;
 
-	if (extent_op && extent_op->update_key)
-		btrfs_disk_key_to_cpu(&tmp_op_key, &extent_op->key);
-
 	spin_lock(&head->lock);
 	for (n = rb_first_cached(&head->ref_tree); n; n = rb_next(n)) {
 		node = rb_entry(n, struct btrfs_delayed_ref_node,
@@ -853,10 +848,16 @@ static int add_delayed_refs(const struct btrfs_fs_info *fs_info,
 		case BTRFS_TREE_BLOCK_REF_KEY: {
 			/* NORMAL INDIRECT METADATA backref */
 			struct btrfs_delayed_tree_ref *ref;
+			struct btrfs_key *key_ptr = NULL;
+
+			if (head->extent_op && head->extent_op->update_key) {
+				btrfs_disk_key_to_cpu(&key, &head->extent_op->key);
+				key_ptr = &key;
+			}
 
 			ref = btrfs_delayed_node_to_tree_ref(node);
 			ret = add_indirect_ref(fs_info, preftrees, ref->root,
-					       &tmp_op_key, ref->level + 1,
+					       key_ptr, ref->level + 1,
 					       node->bytenr, count, sc,
 					       GFP_ATOMIC);
 			break;
-- 
2.35.1




  parent reply	other threads:[~2022-10-27 17:06 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-27 16:55 [PATCH 5.10 00/79] 5.10.151-rc1 review Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 01/79] ocfs2: clear dinode links count in case of error Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 02/79] ocfs2: fix BUG when iput after ocfs2_mknod fails Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 03/79] selinux: enable use of both GFP_KERNEL and GFP_ATOMIC in convert_context() Greg Kroah-Hartman
2022-10-27 16:55   ` Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 04/79] cpufreq: qcom: fix writes in read-only memory region Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 05/79] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 06/79] cpufreq: tegra194: Fix module loading Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 07/79] x86/microcode/AMD: Apply the patch early on every logical thread Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 08/79] hwmon/coretemp: Handle large core ID value Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 09/79] ata: ahci-imx: Fix MODULE_ALIAS Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 10/79] ata: ahci: Match EM_MAX_SLOTS with SATA_PMP_MAX_PORTS Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 11/79] cpufreq: qcom: fix memory leak in error path Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 12/79] kvm: Add support for arch compat vm ioctls Greg Kroah-Hartman
2022-10-30  9:54   ` Pavel Machek
2022-10-27 16:55 ` [PATCH 5.10 13/79] KVM: arm64: vgic: Fix exit condition in scan_its_table() Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 14/79] media: mceusb: set timeout to at least timeout provided Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 15/79] media: venus: dec: Handle the case where find_format fails Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 16/79] bpf: Generate BTF_KIND_FLOAT when linking vmlinux Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 17/79] kbuild: Quote OBJCOPY var to avoid a pahole call break the build Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 18/79] kbuild: skip per-CPU BTF generation for pahole v1.18-v1.21 Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 19/79] kbuild: Unify options for BTF generation for vmlinux and modules Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 20/79] kbuild: Add skip_encoding_btf_enum64 option to pahole Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 21/79] block: wbt: Remove unnecessary invoking of wbt_update_limits in wbt_init Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 22/79] blk-wbt: call rq_qos_add() after wb_normal is initialized Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 23/79] arm64: errata: Remove AES hwcap for COMPAT tasks Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 24/79] r8152: add PID for the Lenovo OneLink+ Dock Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 25/79] btrfs: fix processing of delayed data refs during backref walking Greg Kroah-Hartman
2022-10-27 16:55 ` Greg Kroah-Hartman [this message]
2022-10-27 16:55 ` [PATCH 5.10 27/79] ACPI: extlog: Handle multiple records Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 28/79] tipc: Fix recognition of trial period Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 29/79] tipc: fix an information leak in tipc_topsrv_kern_subscr Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 30/79] i40e: Fix DMA mappings leak Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 31/79] HID: magicmouse: Do not set BTN_MOUSE on double report Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 32/79] sfc: Change VF mac via PF as first preference if available Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 33/79] net/atm: fix proc_mpc_write incorrect return value Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 34/79] net: phy: dp83867: Extend RX strap quirk for SGMII mode Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 35/79] tcp: Add num_closed_socks to struct sock_reuseport Greg Kroah-Hartman
2022-10-27 19:53   ` Kuniyuki Iwashima
2022-10-28  6:17     ` Greg KH
2022-10-28 17:05       ` Kuniyuki Iwashima
2022-10-29  6:27         ` Greg KH
2022-10-27 16:55 ` [PATCH 5.10 36/79] udp: Update reuse->has_conns under reuseport_lock Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 37/79] cifs: Fix xid leak in cifs_copy_file_range() Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 38/79] cifs: Fix xid leak in cifs_flock() Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 39/79] cifs: Fix xid leak in cifs_ses_add_channel() Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 40/79] net: hsr: avoid possible NULL deref in skb_clone() Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 41/79] ionic: catch NULL pointer issue on reconfig Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 42/79] nvme-hwmon: rework to avoid devm allocation Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 43/79] nvme-hwmon: Return error code when registration fails Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 44/79] nvme-hwmon: consistently ignore errors from nvme_hwmon_init Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 45/79] nvme-hwmon: kmalloc the NVME SMART log buffer Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 46/79] net: sched: cake: fix null pointer access issue when cake_init() fails Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 47/79] net: sched: delete duplicate cleanup of backlog and qlen Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 48/79] net: sched: sfb: fix null pointer access issue when sfb_init() fails Greg Kroah-Hartman
2022-10-27 16:55 ` [PATCH 5.10 49/79] sfc: include vport_id in filter spec hash and equal() Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 50/79] net: hns: fix possible memory leak in hnae_ae_register() Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 51/79] net: sched: fix race condition in qdisc_graft() Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 52/79] net: phy: dp83822: disable MDI crossover status change interrupt Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 53/79] iommu/vt-d: Allow NVS regions in arch_rmrr_sanity_check() Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 54/79] iommu/vt-d: Clean up si_domain in the init_dmars() error path Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 55/79] drm/virtio: Use appropriate atomic state in virtio_gpu_plane_cleanup_fb() Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 56/79] dmaengine: mxs-dma: Remove the unused .id_table Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 57/79] dmaengine: mxs: use platform_driver_register Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 58/79] tracing: Simplify conditional compilation code in tracing_set_tracer() Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 59/79] tracing: Do not free snapshot if tracer is on cmdline Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 60/79] xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 61/79] xen/gntdev: Accommodate VMA splitting Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 62/79] mmc: sdhci-tegra: Use actual clock rate for SW tuning correction Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 63/79] riscv: Add machine name to kernel boot log and stack dump output Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 64/79] riscv: always honor the CONFIG_CMDLINE_FORCE when parsing dtb Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 65/79] perf pmu: Validate raw event with sysfs exported format bits Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 66/79] perf: Skip and warn on unknown format configN attrs Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 67/79] fcntl: make F_GETOWN(EX) return 0 on dead owner task Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 68/79] fcntl: fix potential deadlocks for &fown_struct.lock Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 69/79] arm64: dts: qcom: sc7180-trogdor: Fixup modem memory region Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 70/79] arm64: topology: move store_cpu_topology() to shared code Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 71/79] riscv: topology: fix default topology reporting Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 72/79] perf/x86/intel/pt: Relax address filter validation Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 73/79] hv_netvsc: Fix race between VF offering and VF association message from host Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 74/79] [PATCH v3] ACPI: video: Force backlight native for more TongFang devices Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 75/79] x86/Kconfig: Drop check for -mabi=ms for CONFIG_EFI_STUB Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 76/79] Makefile.debug: re-enable debug info for .S files Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 77/79] mmc: core: Add SD card quirk for broken discard Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 78/79] blk-wbt: fix that rwb->wc is always set to 1 in wbt_init() Greg Kroah-Hartman
2022-10-27 16:56 ` [PATCH 5.10 79/79] mm: /proc/pid/smaps_rollup: fix no vmas null-deref Greg Kroah-Hartman
2022-10-27 18:10 ` [PATCH 5.10 00/79] 5.10.151-rc1 review Guenter Roeck
2022-10-27 19:25   ` Greg Kroah-Hartman
2022-10-27 19:27     ` Pavel Machek
2022-10-27 19:39       ` Guenter Roeck
2022-10-27 19:54         ` Florian Fainelli
2022-10-27 19:49       ` Linus Torvalds
2022-10-28 11:01         ` Greg Kroah-Hartman
2022-10-28 10:47 ` Sudip Mukherjee (Codethink)
2022-10-28 10:58   ` Greg Kroah-Hartman
2022-10-28 11:58 ` Jon Hunter
2022-10-28 12:21 ` Pavel Machek
2022-10-28 13:59 ` Naresh Kamboju

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221027165055.277945148@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dsterba@suse.com \
    --cc=fdmanana@suse.com \
    --cc=patches@lists.linux.dev \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.