xfs: fix data corruption w/ unaligned dedupe ranges
authorDave Chinner <dchinner@redhat.com>
Sat, 6 Oct 2018 01:44:19 +0000 (11:44 +1000)
committerDave Chinner <david@fromorbit.com>
Sat, 6 Oct 2018 01:44:19 +0000 (11:44 +1000)
A deduplication data corruption is Exposed by fstests generic/505 on
XFS. It is caused by extending the block match range to include the
partial EOF block, but then allowing unknown data beyond EOF to be
considered a "match" to data in the destination file because the
comparison is only made to the end of the source file. This corrupts
the destination file when the source extent is shared with it.

XFS only supports whole block dedupe, but we still need to appear to
support whole file dedupe correctly.  Hence if the dedupe request
includes the last block of the souce file, don't include it in the
actual XFS dedupe operation. If the rest of the range dedupes
successfully, then report the partial last block as deduped, too, so
that userspace sees it as a successful dedupe rather than return
EINVAL because we can't dedupe unaligned blocks.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
fs/xfs/xfs_reflink.c

index 59da9708e9c1fba08971e20c767ada3a3bb2f444..f889398e25d6afb5a9e246fc7a87b62766c260d4 100644 (file)
@@ -1265,6 +1265,19 @@ xfs_reflink_zero_posteof(
  * will have the iolock and mmaplock held, the page cache of the out file
  * will be truncated, and any leases on the out file will have been broken.
  * This function borrows heavily from xfs_file_aio_write_checks.
+ *
+ * The VFS allows partial EOF blocks to "match" for dedupe even though it hasn't
+ * checked that the bytes beyond EOF physically match. Hence we cannot use the
+ * EOF block in the source dedupe range because it's not a complete block match,
+ * hence can introduce a corruption into the file that has it's
+ * block replaced.
+ *
+ * Despite this issue, we still need to report that range as successfully
+ * deduped to avoid confusing userspace with EINVAL errors on completely
+ * matching file data. The only time that an unaligned length will be passed to
+ * us is when it spans the EOF block of the source file, so if we simply mask it
+ * down to be block aligned here the we will dedupe everything but that partial
+ * EOF block.
  */
 STATIC int
 xfs_reflink_remap_prep(
@@ -1307,6 +1320,14 @@ xfs_reflink_remap_prep(
        if (ret <= 0)
                goto out_unlock;
 
+       /*
+        * If the dedupe data matches, chop off the partial EOF block
+        * from the source file so we don't try to dedupe the partial
+        * EOF block.
+        */
+       if (is_dedupe)
+               *len &= ~((u64)i_blocksize(inode_in) - 1);
+
        /* Attach dquots to dest inode before changing block map */
        ret = xfs_qm_dqattach(dest);
        if (ret)