On Tue, Dec 20, 2022 at 2:51 AM Zbigniew Halas <zhalas@xxxxxxxxx> wrote:
>
> Hi,
>
> I noticed a strange and misleading edge case in FIDEDUPERANGE ioctl.
> For the files with the following content:
> f1: abcd
> f2: efghi
> FIDEDUPERANGE claims to succeed and reports 4 bytes deduplicated,
> despite the files being clearly different. Strace output:
> openat(AT_FDCWD, "f1", O_RDONLY|O_CLOEXEC) = 3
> openat(AT_FDCWD, "f2", O_WRONLY|O_CLOEXEC) = 4
> ioctl(3, BTRFS_IOC_FILE_EXTENT_SAME or FIDEDUPERANGE, {src_offset=0,
> src_length=4, dest_count=1, info=[{dest_fd=4, dest_offset=0}]} =>
> {info=[{bytes_deduped=4, status=0}]}) = 0
>
> The reason is that generic_remap_checks function is doing block
> alignment of the deduplication length when the end of the destination
> file is not at the end of the deduplication range (as described in the
> comment):
> /*
> * If the user wanted us to link to the infile's EOF, round up to the
> * next block boundary for this check.
> *
> * Otherwise, make sure the count is also block-aligned, having
> * already confirmed the starting offsets' block alignment.
> */
>
> So it effectively becomes a zero-length deduplication, which succeeds.
> Despite that it's reported as a successful 4 bytes deduplication.
Ouch!
>
> For a very similar test case, but with the files of the same length:
> f3: abcd
> f4: efgh
> FIDEDUPERANGE fails with FILE_DEDUPE_RANGE_DIFFERS. Strace output:
> openat(AT_FDCWD, "f3", O_RDONLY|O_CLOEXEC) = 3
> openat(AT_FDCWD, "f4", O_WRONLY|O_CLOEXEC) = 4
> ioctl(3, BTRFS_IOC_FILE_EXTENT_SAME or FIDEDUPERANGE, {src_offset=0,
> src_length=4, dest_count=1, info=[{dest_fd=4, dest_offset=0}]} =>
> {info=[{bytes_deduped=0, status=1}]}) = 0
>
> For this case generic_remap_checks does not alter deduplication
> length, and deduplication fails when comparing the content of the
> files.
>
Thanks for the analysis.
Would you be interested in trying to fix the bug and writing a test?
I can help if you would like.
It's hard to follow all the changes since
54dbc1517237 ("vfs: hoist the btrfs deduplication ioctl to the vfs")
in v4.5, but it *looks* like this behavior might have been in btrfs,
before the ioctl was promoted to vfs.. not sure.
We have fstests coverage for the "good" case of same size src/dst
(generic/136), but I didn't find a test for the non-same size src/dst.
In any case, vfs_dedupe_file_range_one() and ->remap_file_range()
do not even have an interface to return the actual bytes_deduped,
so I do not see how any of the REMAP_FILE_CAN_SHORTEN cases
are valid, regardless of EOF.
What am I missing?
Thanks,
Amir.
