diff_cb() does not handle large dnodes

Trying to 'zfs diff' a snapshot with large dnodes will incorrectly try
to access its interior slots when dnodesize > sizeof(dnode_phys_t).
This is normally not an issue because the interior slots are
zero-filled, which report_dnode() handles calling
report_free_dnode_range(). However this is not the case for encrypted
large dnodes or filesystem using many SA based xattrs where the extra
data past the legacy dnode size boundary is interpreted as a
dnode_phys_t.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #7678
Closes #8931
Closes #9343

diff --git a/module/zfs/dmu_diff.c b/module/zfs/dmu_diff.c
index 180f90f94949e5a4b316b0bf90da602982b2e385..c40ed57f243d8f0ba1dcfe57d7151afd16a8a13f 100644 (file)
--- a/module/zfs/dmu_diff.c
+++ b/module/zfs/dmu_diff.c
@@ -21,6 +21,7 @@
 /*
  * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2012, 2018 by Delphix. All rights reserved.
+ * Copyright (c) 2019, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
  */
 
 #include <sys/dmu.h>
@@ -131,7 +132,7 @@ diff_cb(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
                dnode_phys_t *blk;
                arc_buf_t *abuf;
                arc_flags_t aflags = ARC_FLAG_WAIT;
-               int blksz = BP_GET_LSIZE(bp);
+               int epb = BP_GET_LSIZE(bp) >> DNODE_SHIFT;
                int zio_flags = ZIO_FLAG_CANFAIL;
                int i;
 
@@ -143,7 +144,7 @@ diff_cb(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
                        return (SET_ERROR(EIO));
 
                blk = abuf->b_data;
-               for (i = 0; i < blksz >> DNODE_SHIFT; i++) {
+               for (i = 0; i < epb; i += blk[i].dn_extra_slots + 1) {
                        uint64_t dnobj = (zb->zb_blkid <<
                            (DNODE_BLOCK_SHIFT - DNODE_SHIFT)) + i;
                        err = report_dnode(da, dnobj, blk+i);

diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_diff/zfs_diff_encrypted.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_diff/zfs_diff_encrypted.ksh
index 471e9ca68e7e667f08b04a273aa8a80a71d13a1f..96e6d9b5ae89a421f057d7f7922a8238a89898c4 100755 (executable)
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_diff/zfs_diff_encrypted.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_diff/zfs_diff_encrypted.ksh
@@ -24,14 +24,15 @@
 # 1. Create an encrypted dataset
 # 2. Create two snapshots of the dataset
 # 3. Perform 'zfs diff -Ft' and verify no errors occur
+# 4. Perform the same test on a dataset with large dnodes
 #
 
 verify_runnable "both"
 
 function cleanup
 {
-       datasetexists $TESTPOOL/$TESTFS1 && \
-               log_must zfs destroy -r $TESTPOOL/$TESTFS1
+       destroy_dataset "$TESTPOOL/$TESTFS1" "-r"
+       destroy_dataset "$TESTPOOL/$TESTFS2" "-r"
 }
 
 log_assert "'zfs diff' should work with encrypted datasets"
@@ -50,4 +51,13 @@ log_must zfs snapshot $TESTPOOL/$TESTFS1@snap2
 # 3. Perform 'zfs diff' and verify no errors occur
 log_must zfs diff -Ft $TESTPOOL/$TESTFS1@snap1 $TESTPOOL/$TESTFS1@snap2
 
+# 4. Perform the same test on a dataset with large dnodes
+log_must eval "echo 'password' | zfs create -o dnodesize=4k \
+       -o encryption=on -o keyformat=passphrase $TESTPOOL/$TESTFS2"
+MNTPOINT="$(get_prop mountpoint $TESTPOOL/$TESTFS2)"
+log_must zfs snapshot $TESTPOOL/$TESTFS2@snap1
+log_must touch "$MNTPOINT/file"
+log_must zfs snapshot $TESTPOOL/$TESTFS2@snap2
+log_must zfs diff -Ft $TESTPOOL/$TESTFS2@snap1 $TESTPOOL/$TESTFS2@snap2
+
 log_pass "'zfs diff' works with encrypted datasets"