From ec5d76e853a4b040fd76d803d51a40f872065c09 Mon Sep 17 00:00:00 2001 From: loli10K Date: Tue, 24 Sep 2019 21:01:37 +0200 Subject: [PATCH] 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 Reviewed-by: Tom Caputi Reviewed-by: Ryan Moeller Signed-off-by: loli10K Closes #7678 Closes #8931 Closes #9343 --- module/zfs/dmu_diff.c | 5 +++-- .../cli_root/zfs_diff/zfs_diff_encrypted.ksh | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/module/zfs/dmu_diff.c b/module/zfs/dmu_diff.c index 76c32b126..6a7cd844c 100644 --- 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, 2015 by Delphix. All rights reserved. + * Copyright (c) 2019, loli10K . All rights reserved. */ #include @@ -130,7 +131,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; @@ -142,7 +143,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 471e9ca68..96e6d9b5a 100755 --- 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"