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
This commit is contained in:
loli10K 2019-09-24 21:01:37 +02:00 committed by Tony Hutter
parent 8498a2f3f8
commit ec5d76e853
2 changed files with 15 additions and 4 deletions

View File

@ -21,6 +21,7 @@
/* /*
* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2015 by Delphix. All rights reserved. * Copyright (c) 2012, 2015 by Delphix. All rights reserved.
* Copyright (c) 2019, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
*/ */
#include <sys/dmu.h> #include <sys/dmu.h>
@ -130,7 +131,7 @@ diff_cb(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
dnode_phys_t *blk; dnode_phys_t *blk;
arc_buf_t *abuf; arc_buf_t *abuf;
arc_flags_t aflags = ARC_FLAG_WAIT; 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 zio_flags = ZIO_FLAG_CANFAIL;
int i; int i;
@ -142,7 +143,7 @@ diff_cb(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
return (SET_ERROR(EIO)); return (SET_ERROR(EIO));
blk = abuf->b_data; 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 << uint64_t dnobj = (zb->zb_blkid <<
(DNODE_BLOCK_SHIFT - DNODE_SHIFT)) + i; (DNODE_BLOCK_SHIFT - DNODE_SHIFT)) + i;
err = report_dnode(da, dnobj, blk+i); err = report_dnode(da, dnobj, blk+i);

View File

@ -24,14 +24,15 @@
# 1. Create an encrypted dataset # 1. Create an encrypted dataset
# 2. Create two snapshots of the dataset # 2. Create two snapshots of the dataset
# 3. Perform 'zfs diff -Ft' and verify no errors occur # 3. Perform 'zfs diff -Ft' and verify no errors occur
# 4. Perform the same test on a dataset with large dnodes
# #
verify_runnable "both" verify_runnable "both"
function cleanup function cleanup
{ {
datasetexists $TESTPOOL/$TESTFS1 && \ destroy_dataset "$TESTPOOL/$TESTFS1" "-r"
log_must zfs destroy -r $TESTPOOL/$TESTFS1 destroy_dataset "$TESTPOOL/$TESTFS2" "-r"
} }
log_assert "'zfs diff' should work with encrypted datasets" 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 # 3. Perform 'zfs diff' and verify no errors occur
log_must zfs diff -Ft $TESTPOOL/$TESTFS1@snap1 $TESTPOOL/$TESTFS1@snap2 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" log_pass "'zfs diff' works with encrypted datasets"