mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Cc: Robbie Ko <robbieko@synology.com>,
	Filipe Manana <fdmanana@suse.com>,
	"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
Subject: [PATCH review for 4.9 16/50] Btrfs: send, fix failure to rename top level inode due to name collision
Date: Mon, 25 Sep 2017 01:12:45 +0000	[thread overview]
Message-ID: <20170925011225.10029-16-alexander.levin@verizon.com> (raw)
In-Reply-To: <20170925011225.10029-1-alexander.levin@verizon.com>

From: Robbie Ko <robbieko@synology.com>

[ Upstream commit 4dd9920d991745c4a16f53a8f615f706fbe4b3f7 ]

Under certain situations, an incremental send operation can fail due to a
premature attempt to create a new top level inode (a direct child of the
subvolume/snapshot root) whose name collides with another inode that was
removed from the send snapshot.

Consider the following example scenario.

Parent snapshot:

  .                 (ino 256, gen 8)
  |---- a1/         (ino 257, gen 9)
  |---- a2/         (ino 258, gen 9)

Send snapshot:

  .                 (ino 256, gen 3)
  |---- a2/         (ino 257, gen 7)

In this scenario, when receiving the incremental send stream, the btrfs
receive command fails like this (ran in verbose mode, -vv argument):

  rmdir a1
  mkfile o257-7-0
  rename o257-7-0 -> a2
  ERROR: rename o257-7-0 -> a2 failed: Is a directory

What happens when computing the incremental send stream is:

1) An operation to remove the directory with inode number 257 and
   generation 9 is issued.

2) An operation to create the inode with number 257 and generation 7 is
   issued. This creates the inode with an orphanized name of "o257-7-0".

3) An operation rename the new inode 257 to its final name, "a2", is
   issued. This is incorrect because inode 258, which has the same name
   and it's a child of the same parent (root inode 256), was not yet
   processed and therefore no rmdir operation for it was yet issued.
   The rename operation is issued because we fail to detect that the
   name of the new inode 257 collides with inode 258, because their
   parent, a subvolume/snapshot root (inode 256) has a different
   generation in both snapshots.

So fix this by ignoring the generation value of a parent directory that
matches a root inode (number 256) when we are checking if the name of the
inode currently being processed collides with the name of some other
inode that was not yet processed.

We can achieve this scenario of different inodes with the same number but
different generation values either by mounting a filesystem with the inode
cache option (-o inode_cache) or by creating and sending snapshots across
different filesystems, like in the following example:

  $ mkfs.btrfs -f /dev/sdb
  $ mount /dev/sdb /mnt
  $ mkdir /mnt/a1
  $ mkdir /mnt/a2
  $ btrfs subvolume snapshot -r /mnt /mnt/snap1
  $ btrfs send /mnt/snap1 -f /tmp/1.snap
  $ umount /mnt

  $ mkfs.btrfs -f /dev/sdc
  $ mount /dev/sdc /mnt
  $ touch /mnt/a2
  $ btrfs subvolume snapshot -r /mnt /mnt/snap2
  $ btrfs receive /mnt -f /tmp/1.snap
  # Take note that once the filesystem is created, its current
  # generation has value 7 so the inode from the second snapshot has
  # a generation value of 7. And after receiving the first snapshot
  # the filesystem is at a generation value of 10, because the call to
  # create the second snapshot bumps the generation to 8 (the snapshot
  # creation ioctl does a transaction commit), the receive command calls
  # the snapshot creation ioctl to create the first snapshot, which bumps
  # the filesystem's generation to 9, and finally when the receive
  # operation finishes it calls an ioctl to transition the first snapshot
  # (snap1) from RW mode to RO mode, which does another transaction commit
  # and bumps the filesystem's generation to 10.
  $ rm -f /tmp/1.snap
  $ btrfs send /mnt/snap1 -f /tmp/1.snap
  $ btrfs send -p /mnt/snap1 /mnt/snap2 -f /tmp/2.snap
  $ umount /mnt

  $ mkfs.btrfs -f /dev/sdd
  $ mount /dev/sdd /mnt
  $ btrfs receive /mnt /tmp/1.snap
  # Receive of snapshot snap2 used to fail.
  $ btrfs receive /mnt /tmp/2.snap

Signed-off-by: Robbie Ko <robbieko@synology.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
[Rewrote changelog to be more precise and clear]
Signed-off-by: Filipe Manana <fdmanana@suse.com>

Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 fs/btrfs/send.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 71261b459863..77f9efc1f7aa 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1680,6 +1680,9 @@ static int is_inode_existent(struct send_ctx *sctx, u64 ino, u64 gen)
 {
 	int ret;
 
+	if (ino == BTRFS_FIRST_FREE_OBJECTID)
+		return 1;
+
 	ret = get_cur_inode_state(sctx, ino, gen);
 	if (ret < 0)
 		goto out;
@@ -1865,7 +1868,7 @@ static int will_overwrite_ref(struct send_ctx *sctx, u64 dir, u64 dir_gen,
 	 * not deleted and then re-created, if it was then we have no overwrite
 	 * and we can just unlink this entry.
 	 */
-	if (sctx->parent_root) {
+	if (sctx->parent_root && dir != BTRFS_FIRST_FREE_OBJECTID) {
 		ret = get_inode_info(sctx->parent_root, dir, NULL, &gen, NULL,
 				     NULL, NULL, NULL);
 		if (ret < 0 && ret != -ENOENT)
-- 
2.11.0

  parent reply	other threads:[~2017-09-25  1:30 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-25  1:12 [PATCH review for 4.9 01/50] xen-netback: Use GFP_ATOMIC to allocate hash Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 02/50] locking/lockdep: Add nest_lock integrity test Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 04/50] irqchip/crossbar: Fix incorrect type of local variables Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 03/50] watchdog: kempld: fix gcc-4.3 build Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 06/50] mac80211_hwsim: check HWSIM_ATTR_RADIO_NAME length Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 05/50] initramfs: finish fput() before accessing any binary from initramfs Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 07/50] ALSA: hda: Add Geminilake HDMI codec ID Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 08/50] qed: Don't use attention PTT for configuring BW Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 09/50] mac80211: fix power saving clients handling in iwlwifi Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 12/50] netfilter: nf_ct_expect: Change __nf_ct_expect_check() return value Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 11/50] staging: vchiq_2835_arm: Make cache-line-size a required DT property Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 10/50] net/mlx4_en: fix overflow in mlx4_en_init_timestamp() Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 13/50] iio: adc: xilinx: Fix error handling Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 14/50] f2fs: do SSR for data when there is enough free space Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 15/50] sched/fair: Update rq clock before changing a task's CPU affinity Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 17/50] f2fs: do not wait for writeback in write_begin Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` Levin, Alexander (Sasha Levin) [this message]
2017-09-25  1:12 ` [PATCH review for 4.9 18/50] md/linear: shutup lockdep warnning Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 19/50] sparc64: Migrate hvcons irq to panicked cpu Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 20/50] net/mlx4_core: Fix VF overwrite of module param which disables DMFS on new probed PFs Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 21/50] crypto: xts - Add ECB dependency Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 22/50] mm/memory_hotplug: set magic number to page->freelist instead of page->lru.next Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 23/50] ocfs2/dlmglue: prepare tracking logic to avoid recursive cluster lock Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 24/50] slub: do not merge cache if slub_debug contains a never-merge flag Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 27/50] drm/amdgpu: refuse to reserve io mem for split VRAM buffers Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 26/50] ASoC: mediatek: add I2C dependency for CS42XX8 Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 25/50] scsi: scsi_dh_emc: return success in clariion_std_inquiry() Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 28/50] net: mvpp2: release reference to txq_cpu[] entry after unmapping Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 30/50] qed: Reserve doorbell BAR space for present CPUs Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 29/50] qede: Prevent index problems in loopback test Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 31/50] qed: Read queue state before releasing buffer Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 32/50] i2c: at91: ensure state is restored after suspending Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 33/50] ceph: don't update_dentry_lease unless we actually got one Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 34/50] ceph: fix bogus endianness change in ceph_ioctl_set_layout Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 35/50] ceph: clean up unsafe d_parent accesses in build_dentry_path Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 36/50] uapi: fix linux/rds.h userspace compilation errors Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 39/50] IB/hfi1: Use static CTLE with Preset 6 for integrated HFIs Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 37/50] uapi: fix linux/mroute6.h userspace compilation errors Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 38/50] ibmvnic: Handle processing of CRQ messages in a tasklet Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 40/50] IB/hfi1: Allocate context data on memory node Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 41/50] target/iscsi: Fix unsolicited data seq_end_offset calculation Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 42/50] hrtimer: Catch invalid clockids again Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 43/50] nfsd/callback: Cleanup callback cred on shutdown Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 44/50] powerpc/perf: Add restrictions to PMC5 in power9 DD1 Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 45/50] drm/nouveau/gr/gf100-: fix ccache error logging Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 47/50] btmrvl: avoid double-disable_irq() race Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 46/50] regulator: core: Resolve supplies before disabling unused regulators Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 48/50] EDAC, mce_amd: Print IPID and Syndrome on a separate line Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 50/50] cpufreq: CPPC: add ACPI_PROCESSOR dependency Levin, Alexander (Sasha Levin)
2017-09-25  1:12 ` [PATCH review for 4.9 49/50] genirq: Clarify logic calculating bogus irqreturn_t values Levin, Alexander (Sasha Levin)

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=20170925011225.10029-16-alexander.levin@verizon.com \
    --to=alexander.levin@verizon.com \
    --cc=fdmanana@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robbieko@synology.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®