[PATCH 4.14 096/156] Btrfs: incremental send, fix wrong unlink path after renaming file

2018-02-02 Thread Greg Kroah-Hartman
4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Filipe Manana 


[ Upstream commit ea37d5998b50a72b9045ba60a132eeb20e1c4230 ]

Under some circumstances, an incremental send operation can issue wrong
paths for unlink commands related to files that have multiple hard links
and some (or all) of those links were renamed between the parent and send
snapshots. Consider the following example:

Parent snapshot

 .  (ino 256)
 | a/   (ino 257)
 | | b/ (ino 259)
 | | | c/   (ino 260)
 | | | f2   (ino 261)
 | |
 | | f2l1   (ino 261)
 |
 | d/   (ino 262)
   | f1l1_2 (ino 258)
   | f2l2   (ino 261)
   | f1_2   (ino 258)

Send snapshot

 .  (ino 256)
 | a/   (ino 257)
 | | f2l1/  (ino 263)
 | | b2/(ino 259)
 |   | c/   (ino 260)
 |   | | d3 (ino 262)
 |   |   | f1l1_2   (ino 258)
 |   |   | f2l2_2   (ino 261)
 |   |   | f1_2 (ino 258)
 |   |
 |   | f2   (ino 261)
 |   | f1l2 (ino 258)
 |
 | d(ino 261)

When computing the incremental send stream the following steps happen:

1) When processing inode 261, a rename operation is issued that renames
   inode 262, which currently as a path of "d", to an orphan name of
   "o262-7-0". This is done because in the send snapshot, inode 261 has
   of its hard links with a path of "d" as well.

2) Two link operations are issued that create the new hard links for
   inode 261, whose names are "d" and "f2l2_2", at paths "/" and
   "o262-7-0/" respectively.

3) Still while processing inode 261, unlink operations are issued to
   remove the old hard links of inode 261, with names "f2l1" and "f2l2",
   at paths "a/" and "d/". However path "d/" does not correspond anymore
   to the directory inode 262 but corresponds instead to a hard link of
   inode 261 (link command issued in the previous step). This makes the
   receiver fail with a ENOTDIR error when attempting the unlink
   operation.

The problem happens because before sending the unlink operation, we failed
to detect that inode 262 was one of ancestors for inode 261 in the parent
snapshot, and therefore we didn't recompute the path for inode 262 before
issuing the unlink operation for the link named "f2l2" of inode 262. The
detection failed because the function "is_ancestor()" only follows the
first hard link it finds for an inode instead of all of its hard links
(as it was originally created for being used with directories only, for
which only one hard link exists). So fix this by making "is_ancestor()"
follow all hard links of the input inode.

A test case for fstests follows soon.

Signed-off-by: Filipe Manana 
Signed-off-by: David Sterba 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 fs/btrfs/send.c |  124 +++-
 1 file changed, 106 insertions(+), 18 deletions(-)

--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3527,7 +3527,40 @@ out:
 }
 
 /*
- * Check if ino ino1 is an ancestor of inode ino2 in the given root.
+ * Check if inode ino2, or any of its ancestors, is inode ino1.
+ * Return 1 if true, 0 if false and < 0 on error.
+ */
+static int check_ino_in_path(struct btrfs_root *root,
+const u64 ino1,
+const u64 ino1_gen,
+const u64 ino2,
+const u64 ino2_gen,
+struct fs_path *fs_path)
+{
+   u64 ino = ino2;
+
+   if (ino1 == ino2)
+   return ino1_gen == ino2_gen;
+
+   while (ino > BTRFS_FIRST_FREE_OBJECTID) {
+   u64 parent;
+   u64 parent_gen;
+   int ret;
+
+   fs_path_reset(fs_path);
+   ret = get_first_ref(root, ino, , _gen, fs_path);
+   if (ret < 0)
+   return ret;
+   if (parent == ino1)
+   return 

[PATCH 4.14 096/156] Btrfs: incremental send, fix wrong unlink path after renaming file

2018-02-02 Thread Greg Kroah-Hartman
4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Filipe Manana 


[ Upstream commit ea37d5998b50a72b9045ba60a132eeb20e1c4230 ]

Under some circumstances, an incremental send operation can issue wrong
paths for unlink commands related to files that have multiple hard links
and some (or all) of those links were renamed between the parent and send
snapshots. Consider the following example:

Parent snapshot

 .  (ino 256)
 | a/   (ino 257)
 | | b/ (ino 259)
 | | | c/   (ino 260)
 | | | f2   (ino 261)
 | |
 | | f2l1   (ino 261)
 |
 | d/   (ino 262)
   | f1l1_2 (ino 258)
   | f2l2   (ino 261)
   | f1_2   (ino 258)

Send snapshot

 .  (ino 256)
 | a/   (ino 257)
 | | f2l1/  (ino 263)
 | | b2/(ino 259)
 |   | c/   (ino 260)
 |   | | d3 (ino 262)
 |   |   | f1l1_2   (ino 258)
 |   |   | f2l2_2   (ino 261)
 |   |   | f1_2 (ino 258)
 |   |
 |   | f2   (ino 261)
 |   | f1l2 (ino 258)
 |
 | d(ino 261)

When computing the incremental send stream the following steps happen:

1) When processing inode 261, a rename operation is issued that renames
   inode 262, which currently as a path of "d", to an orphan name of
   "o262-7-0". This is done because in the send snapshot, inode 261 has
   of its hard links with a path of "d" as well.

2) Two link operations are issued that create the new hard links for
   inode 261, whose names are "d" and "f2l2_2", at paths "/" and
   "o262-7-0/" respectively.

3) Still while processing inode 261, unlink operations are issued to
   remove the old hard links of inode 261, with names "f2l1" and "f2l2",
   at paths "a/" and "d/". However path "d/" does not correspond anymore
   to the directory inode 262 but corresponds instead to a hard link of
   inode 261 (link command issued in the previous step). This makes the
   receiver fail with a ENOTDIR error when attempting the unlink
   operation.

The problem happens because before sending the unlink operation, we failed
to detect that inode 262 was one of ancestors for inode 261 in the parent
snapshot, and therefore we didn't recompute the path for inode 262 before
issuing the unlink operation for the link named "f2l2" of inode 262. The
detection failed because the function "is_ancestor()" only follows the
first hard link it finds for an inode instead of all of its hard links
(as it was originally created for being used with directories only, for
which only one hard link exists). So fix this by making "is_ancestor()"
follow all hard links of the input inode.

A test case for fstests follows soon.

Signed-off-by: Filipe Manana 
Signed-off-by: David Sterba 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 fs/btrfs/send.c |  124 +++-
 1 file changed, 106 insertions(+), 18 deletions(-)

--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3527,7 +3527,40 @@ out:
 }
 
 /*
- * Check if ino ino1 is an ancestor of inode ino2 in the given root.
+ * Check if inode ino2, or any of its ancestors, is inode ino1.
+ * Return 1 if true, 0 if false and < 0 on error.
+ */
+static int check_ino_in_path(struct btrfs_root *root,
+const u64 ino1,
+const u64 ino1_gen,
+const u64 ino2,
+const u64 ino2_gen,
+struct fs_path *fs_path)
+{
+   u64 ino = ino2;
+
+   if (ino1 == ino2)
+   return ino1_gen == ino2_gen;
+
+   while (ino > BTRFS_FIRST_FREE_OBJECTID) {
+   u64 parent;
+   u64 parent_gen;
+   int ret;
+
+   fs_path_reset(fs_path);
+   ret = get_first_ref(root, ino, , _gen, fs_path);
+   if (ret < 0)
+   return ret;
+   if (parent == ino1)
+   return parent_gen == ino1_gen;
+   ino = parent;
+   }
+   return 0;
+}
+
+/*
+ * Check if ino ino1 is an