On Sun, May 3, 2015 at 2:56 AM, Filipe Manana <fdmanana@xxxxxxxx> wrote:
> Marc reported a problem where the receiving end of an incremental send
> was performing clone operations that failed with -EINVAL. This happened
> because, unlike for uncompressed extents, we were not checking if the
> source clone offset and length, after summing the data offset, falls
> within the source file's boundaries.
>
> So make sure we do such checks when attempting to issue clone operations
> for compressed extents.
>
> Problem reproducible with the following steps:
>
> $ mkfs.btrfs -f /dev/sdb
> $ mount -o compress /dev/sdb /mnt
> $ mkfs.btrfs -f /dev/sdc
> $ mount -o compress /dev/sdc /mnt2
>
> # Create the file with a single extent of 128K. This creates a metadata file
> # extent item with a data start offset of 0 and a logical length of 128K.
> $ xfs_io -f -c "pwrite -S 0xaa 64K 128K" -c "fsync" /mnt/foo
>
> # Now rewrite the range 64K to 112K of our file. This will make the inode's
> # metadata continue to point to the 128K extent we created before, but now
> # with an extent item that points to the extent with a data start offset of
> # 112K and a logical length of 16K.
> # That metadata file extent item is associated with the logical file offset
> # at 176K and covers the logical file range 176K to 192K.
> $ xfs_io -c "pwrite -S 0xbb 64K 112K" -c "fsync" /mnt/foo
>
> # Now rewrite the range 180K to 12K. This will make the inode's metadata
> # continue to point the the 128K extent we created earlier, with a single
> # extent item that points to it with a start offset of 112K and a logical
> # length of 4K.
> # That metadata file extent item is associated with the logical file offset
> # at 176K and covers the logical file range 176K to 180K.
> $ xfs_io -c "pwrite -S 0xcc 180K 12K" -c "fsync" /mnt/foo
>
> $ btrfs subvolume snapshot -r /mnt /mnt/snap1
>
> $ touch /mnt/bar
> # Calls the btrfs clone ioctl.
> $ ~/xfstests/src/cloner -s $((176 * 1024)) -d $((176 * 1024)) \
> -l $((4 * 1024)) /mnt/foo /mnt/bar
>
> $ btrfs subvolume snapshot -r /mnt /mnt/snap2
>
> $ btrfs send /mnt/snap1 | btrfs receive /mnt2
> At subvol /mnt/snap1
> At subvol snap1
>
> $ btrfs send -p /mnt/snap1 /mnt/snap2 | btrfs receive /mnt2
> At subvol /mnt/snap2
> At snapshot snap2
> ERROR: failed to clone extents to bar
> Invalid argument
>
> A test case for fstests follows soon.
>
> Reported-by: Marc MERLIN <marc@xxxxxxxxxxx>
> Tested-by: Marc MERLIN <marc@xxxxxxxxxxx>
> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> ---
> fs/btrfs/send.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 895f1b1..50ebc62 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -1159,6 +1159,9 @@ struct backref_ctx {
> /* may be truncated in case it's the last extent in a file */
> u64 extent_len;
>
> + /* data offset in the file extent item */
> + u64 data_offset;
> +
> /* Just to check for bugs in backref resolving */
> int found_itself;
> };
> @@ -1222,7 +1225,7 @@ static int __iterate_backrefs(u64 ino, u64 offset, u64 root, void *ctx_)
> if (ret < 0)
> return ret;
>
> - if (offset + bctx->extent_len > i_size)
> + if (offset + bctx->data_offset + bctx->extent_len > i_size)
> return 0;
>
> /*
> @@ -1364,6 +1367,19 @@ static int find_extent_clone(struct send_ctx *sctx,
> backref_ctx->cur_offset = data_offset;
> backref_ctx->found_itself = 0;
> backref_ctx->extent_len = num_bytes;
> + /*
> + * For non-compressed extents iterate_extent_inodes() gives us extent
> + * offsets that already take into account the data offset, but not for
> + * compressed extents, since the offset is logical and not relative to
> + * the physical extent locations. We must take this into account to
> + * avoid sending clone offsets that go beyond the source file's size,
> + * which would result in the clone ioctl failing with -EINVAL on the
> + * receiving end.
> + */
> + if (compressed == BTRFS_COMPRESS_NONE)
> + backref_ctx->data_offset = 0;
> + else
> + backref_ctx->data_offset = btrfs_file_extent_offset(eb, fi);
>
> /*
> * The last extent of a file may be too large due to page alignment.
> --
> 2.1.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Is this patch still useful? It applies to 4.1.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
