From: Filipe Manana Date: Tue, 13 Jun 2017 13:13:11 +0000 (+0100) Subject: Btrfs: incremental send, fix invalid path for unlink commands X-Git-Url: https://git.stricted.de/?a=commitdiff_plain;h=fdb1388994e2ed0e1512006244022c721c650c5b;p=GitHub%2Fmoto-9609%2Fandroid_kernel_motorola_exynos9610.git Btrfs: incremental send, fix invalid path for unlink commands An incremental send can contain unlink operations with an invalid target path when we rename some directory inode A, then rename some file inode B to the old name of inode A and directory inode A is an ancestor of inode B in the parent snapshot (but not anymore in the send snapshot). Consider the following example scenario where this issue happens. Parent snapshot: . (ino 256) | |--- dir1/ (ino 257) |--- dir2/ (ino 258) | |--- file1 (ino 259) | |--- file3 (ino 261) | |--- dir3/ (ino 262) |--- file22 (ino 260) |--- dir4/ (ino 263) Send snapshot: . (ino 256) | |--- dir1/ (ino 257) |--- dir2/ (ino 258) |--- dir3 (ino 260) |--- file3/ (ino 262) |--- dir4/ (ino 263) |--- file11 (ino 269) |--- file33 (ino 261) When attempting to apply the corresponding incremental send stream, an unlink operation contains an invalid path which makes the receiver fail. The following is verbose output of the btrfs receive command: receiving snapshot snap2 uuid=7d5450da-a573-e043-a451-ec85f4879f0f (...) utimes utimes dir1 utimes dir1/dir2 link dir1/dir3/dir4/file11 -> dir1/dir2/file1 unlink dir1/dir2/file1 utimes dir1/dir2 truncate dir1/dir3/dir4/file11 size=0 utimes dir1/dir3/dir4/file11 rename dir1/dir3 -> o262-7-0 link dir1/dir3 -> o262-7-0/file22 unlink dir1/dir3/file22 ERROR: unlink dir1/dir3/file22 failed. Not a directory The following steps happen during the computation of the incremental send stream the lead to this issue: 1) Before we start processing the new and deleted references for inode 260, we compute the full path of the deleted reference ("dir1/dir3/file22") and cache it in the list of deleted references for our inode. 2) We then start processing the new references for inode 260, for which there is only one new, located at "dir1/dir3". When processing this new reference, we check that inode 262, which was not yet processed, collides with the new reference and because of that we orphanize inode 262 so its new full path becomes "o262-7-0". 3) After the orphanization of inode 262, we create the new reference for inode 260 by issuing a link command with a target path of "dir1/dir3" and a source path of "o262-7-0/file22". 4) We then start processing the deleted references for inode 260, for which there is only one with the base name of "file22", and issue an unlink operation containing the target path computed at step 1, which is wrong because that path no longer exists and should be replaced with "o262-7-0/file22". So fix this issue by recomputing the full path of deleted references if when we processed the new references for an inode we ended up orphanizing any other inode that is an ancestor of our inode in the parent snapshot. A test case for fstests follows soon. Signed-off-by: Filipe Manana [ adjusted after prev patch removed fs_path::dir_path and dir_path_len ] Signed-off-by: David Sterba --- diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index ddd2b786f4d5..a562dc228794 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -2776,6 +2776,13 @@ struct recorded_ref { int name_len; }; +static void set_ref_path(struct recorded_ref *ref, struct fs_path *path) +{ + ref->full_path = path; + ref->name = (char *)kbasename(ref->full_path->start); + ref->name_len = ref->full_path->end - ref->name; +} + /* * We need to process new refs before deleted refs, but compare_tree gives us * everything mixed. So we first record all refs and later process them. @@ -2792,11 +2799,7 @@ static int __record_ref(struct list_head *head, u64 dir, ref->dir = dir; ref->dir_gen = dir_gen; - ref->full_path = path; - - ref->name = (char *)kbasename(ref->full_path->start); - ref->name_len = ref->full_path->end - ref->name; - + set_ref_path(ref, path); list_add_tail(&ref->list, head); return 0; } @@ -3691,6 +3694,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) int is_orphan = 0; u64 last_dir_ino_rm = 0; bool can_rename = true; + bool orphanized_ancestor = false; btrfs_debug(fs_info, "process_recorded_refs %llu", sctx->cur_ino); @@ -3846,6 +3850,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) ow_inode, ow_gen, sctx->cur_ino, NULL); if (ret > 0) { + orphanized_ancestor = true; fs_path_reset(valid_path); ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, @@ -3971,6 +3976,43 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) if (ret < 0) goto out; if (!ret) { + /* + * If we orphanized any ancestor before, we need + * to recompute the full path for deleted names, + * since any such path was computed before we + * processed any references and orphanized any + * ancestor inode. + */ + if (orphanized_ancestor) { + struct fs_path *new_path; + + /* + * Our reference's name member points to + * its full_path member string, so we + * use here a new path. + */ + new_path = fs_path_alloc(); + if (!new_path) { + ret = -ENOMEM; + goto out; + } + ret = get_cur_path(sctx, cur->dir, + cur->dir_gen, + new_path); + if (ret < 0) { + fs_path_free(new_path); + goto out; + } + ret = fs_path_add(new_path, + cur->name, + cur->name_len); + if (ret < 0) { + fs_path_free(new_path); + goto out; + } + fs_path_free(cur->full_path); + set_ref_path(cur, new_path); + } ret = send_unlink(sctx, cur->full_path); if (ret < 0) goto out;