diff_cb() does not handle encrypted large dnodes #9343

loli10K · 2019-09-22T08:01:36Z

Motivation and Context

Description

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 where raw, unencrypted data is interpreted as a dnode_phys_t.

I am not sure why with encrypted datasets the interior slots are not zero-filled; debugging with sample data shows interior slots being zero-filled only for unencrypted datasets:

Thread 302 hit Breakpoint 1, report_dnode (da=0xffff8801394b7d80, object=2, dnp=0xffff8800af154400) at /usr/src/zfs/module/zfs/dmu_diff.c:86
86	{
(gdb) up
#1  0xffffffffc02458ee in diff_cb (spa=0xffff8800b0348000, zilog=0x0 <irq_stack_union>, bp=0xffffc900012d5000, zb=0xffff8800b865e0c0, dnp=0xffff8800b9678000, arg=0xffff8801394b7d80)
    at /usr/src/zfs/module/zfs/dmu_diff.c:149
149				if (err)
(gdb) x/64x blk+2
0xffff8800af154400:	0x01011113	0x0b00002c	0x00b00001	0x00000001
0xffff8800af154410:	0x00000000	0x00000000	0x00001000	0x00000000
0xffff8800af154420:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154430:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154440:	0x00000008	0x00000000	0x00000470	0x00000000
0xffff8800af154450:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154460:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154470:	0x00000000	0x80130702	0x00000000	0x00000000
0xffff8800af154480:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154490:	0x00000009	0x00000000	0x00000001	0x00000000
0xffff8800af1544a0:	0x0a312031	0x00000000	0x18901880	0x00000005
0xffff8800af1544b0:	0xb04e2c40	0x00000148	0x2d3b7d80	0x000037a3
0xffff8800af1544c0:	0x002f505a	0x00180402	0x000081a4	0x00000000
0xffff8800af1544d0:	0x00000004	0x00000000	0x00000009	0x00000000
0xffff8800af1544e0:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af1544f0:	0x00000022	0x00000000	0x00000004	0x00008408
(gdb) x/64x blk+3
0xffff8800af154600:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154610:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154620:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154630:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154640:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154650:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154660:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154670:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154680:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af154690:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af1546a0:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af1546b0:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af1546c0:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af1546d0:	0x00000000	0x00000000	0x00000encrypted000	0x00000000
0xffff8800af1546e0:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800af1546f0:	0x00000000	0x00000000	0x00000000	0x00000000
(gdb) p *(blk+3)
$3 = {dn_type = 0 '\000', dn_indblkshift = 0 '\000', dn_nlevels = 0 '\000', dn_nblkptr = 0 '\000', dn_bonustype = 0 '\000', dn_checksum = 0 '\000', dn_compress = 0 '\000', dn_flags = 0 '\000', 
  dn_datablkszsec = 0, dn_bonuslen = 0, dn_extra_slots = 0 '\000', dn_pad2 = "\000\000", dn_maxblkid = 0, dn_used = 0, dn_pad3 = {0, 0, 0, 0}, {dn_blkptr = {{blk_dva = {{dva_word = {0, 0}}, {dva_word = {0, 
              0}}, {dva_word = {0, 0}}}, blk_prop = 0, blk_pad = {0, 0}, blk_phys_birth = 0, blk_birth = 0, blk_fill = 0, blk_cksum = {zc_word = {0, 0, 0, 0}}}, {blk_dva = {{dva_word = {0, 0}}, {dva_word = {0, 
              0}}, {dva_word = {0, 0}}}, blk_prop = 0, blk_pad = {0, 0}, blk_phys_birth = 0, blk_birth = 0, blk_fill = 0, blk_cksum = {zc_word = {0, 0, 0, 0}}}, {blk_dva = {{dva_word = {0, 0}}, {dva_word = {0, 
              0}}, {dva_word = {0, 0}}}, blk_prop = 0, blk_pad = {0, 0}, blk_phys_birth = 0, blk_birth = 0, blk_fill = 0, blk_cksum = {zc_word = {0, 0, 0, 0}}}}, {__dn_ignore1 = {blk_dva = {{dva_word = {0, 
              0}}, {dva_word = {0, 0}}, {dva_word = {0, 0}}}, blk_prop = 0, blk_pad = {0, 0}, blk_phys_birth = 0, blk_birth = 0, blk_fill = 0, blk_cksum = {zc_word = {0, 0, 0, 0}}}, 
      dn_bonus = '\000' <repeats 319 times>}, {__dn_ignore2 = {blk_dva = {{dva_word = {0, 0}}, {dva_word = {0, 0}}, {dva_word = {0, 0}}}, blk_prop = 0, blk_pad = {0, 0}, blk_phys_birth = 0, blk_birth = 0, 
        blk_fill = 0, blk_cksum = {zc_word = {0, 0, 0, 0}}}, __dn_ignore3 = '\000' <repeats 191 times>, dn_spill = {blk_dva = {{dva_word = {0, 0}}, {dva_word = {0, 0}}, {dva_word = {0, 0}}}, blk_prop = 0, 
        blk_pad = {0, 0}, blk_phys_birth = 0, blk_birth = 0, blk_fill = 0, blk_cksum = {zc_word = {0, 0, 0, 0}}}}}}

with encryption enabled:

Thread 362 hit Breakpoint 1, report_dnode (da=0xffff8800af17bd80, object=2, dnp=0xffff8800b93dc400) at /usr/src/zfs/module/zfs/dmu_diff.c:86
86	{
(gdb) up
#1  0xffffffffc02458ee in diff_cb (spa=0xffff8800b0348000, zilog=0x0 <irq_stack_union>, bp=0xffffc90001788000, zb=0xffff8800b7ee5d40, dnp=0xffff8800b2295000, arg=0xffff8800af17bd80)
    at /usr/src/zfs/module/zfs/dmu_diff.c:149
149				if (err)
(gdb) x/64x blk+2
0xffff8800b93dc400:	0x01011113	0x0b00002c	0x00b00001	0x00000001
0xffff8800b93dc410:	0x00000000	0x00000000	0x00001000	0x00000000
0xffff8800b93dc420:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b93dc430:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b93dc440:	0x00000008	0x00000000	0x000004d8	0x00000000
0xffff8800b93dc450:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b93dc460:	0xf47d2301	0xd78d52bd	0x0bc548d1	0xe99db3df
0xffff8800b93dc470:	0x00000000	0xa0130702	0x00000000	0x00000000
0xffff8800b93dc480:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b93dc490:	0x00000009	0x00000000	0x00000001	0x2b74482a
0xffff8800b93dc4a0:	0x96f83627	0x0002dad9	0x2f87a660	0x005eaa9f
0xffff8800b93dc4b0:	0xd526860f	0x7f3335b2	0xd9d7b4c3	0x636926c6
0xffff8800b93dc4c0:	0xe2ba0fa9	0xc73ddcca	0x5240693a	0x39b8f538
0xffff8800b93dc4d0:	0x1e90cdba	0xe6d288d5	0xd7c5a711	0x8ae90096
0xffff8800b93dc4e0:	0xf42e30b3	0xd3f4124d	0xcd2dd6b3	0xdfae47bc
0xffff8800b93dc4f0:	0x6c94a732	0x30f79702	0x5ccc91dd	0x000b8e60
(gdb) x/64x blk+3
0xffff8800b93dc600:	0x70b79482	0x3171b6d8	0xc9647367	0xe58fde1e
0xffff8800b93dc610:	0x4afa6f7a	0x516458d1	0xf2d1e632	0xd7a3bd6e
0xffff8800b93dc620:	0xdbf89484	0xe8ceb436	0x54da4e17	0x78f501df
0xffff8800b93dc630:	0xc26c51d1	0x5d4543c6	0x5ce36301	0x0d4d73a3
0xffff8800b93dc640:	0x9671665a	0xd803304a	0x0dc40c3f	0x29cca17f
0xffff8800b93dc650:	0x4a14b163	0x95e45e05	0xe8ee4c15	0xbc77f4ce
0xffff8800b93dc660:	0x4cfece4a	0xf74a5b55	0xa72936c9	0x1e81ff02
0xffff8800b93dc670:	0x61342ecd	0x7cf296c7	0x3ca0db6f	0x4101eb84
0xffff8800b93dc680:	0x0e47430d	0x82f1249d	0x07d1c716	0x6586ad1d
0xffff8800b93dc690:	0x999070f8	0xc63e2126	0xbf6767c8	0x5057b1e0
0xffff8800b93dc6a0:	0xb8de45c5	0x89f08a22	0x8f7c1c04	0x8e1b9213
0xffff8800b93dc6b0:	0x7d674fff	0xafb18ca0	0xef6aea4c	0x44bedce3
0xffff8800b93dc6c0:	0x8240b7ba	0x3c9fc775	0x5ff4f490	0x3299f845
0xffff8800b93dc6d0:	0xd202683f	0xf438de3b	0x15b9ec2c	0xc3b9c7a5
0xffff8800b93dc6e0:	0xd2cb2ea5	0xfc516de9	0xbc2d8821	0x33a793bd
0xffff8800b93dc6f0:	0x3f332a18	0x8d0f8395	0x2c5f8c93	0xb8aa251e
(gdb) p *(blk+3)
$4 = {dn_type = 130 '\202', dn_indblkshift = 148 '\224', dn_nlevels = 183 '\267', dn_nblkptr = 112 'p', dn_bonustype = 216 '\330', dn_checksum = 182 '\266', dn_compress = 113 'q', dn_flags = 49 '1', 
  dn_datablkszsec = 29543, dn_bonuslen = 51556, dn_extra_slots = 30 '\036', dn_pad2 = <incomplete sequence \345>, dn_maxblkid = 5864910270672564090, dn_used = 15538471423576237618, dn_pad3 = {
    16775543809713411204, 8715874712558390807, 6720852539864797649, 958549441175053057}, {dn_blkptr = {{blk_dva = {{dva_word = {15565337834032293466, 3011959817867889727}}, {dva_word = {10800861183152009571, 
              13580592370867391509}}, {dva_word = {17819155297566903882, 2198318479993026249}}}, blk_prop = 9003424388140510925, blk_pad = {4684284040627411823, 9435362951285916429}, 
        blk_phys_birth = 7315724984906794774, blk_birth = 14284891517734646008, blk_fill = 5789291424850077640, blk_cksum = {zc_word = {9939596259341780421, 10239938790466264068, 12660054648441753599, 
            4953639511686572620}}}, {blk_dva = {{dva_word = {4368429471083050938, 3646218350179447952}}, {dva_word = {17598059892458022975, 14103523219935456300}}, {dva_word = {18181434021702741669, 
              3722106060160206881}}}, blk_prop = 10164487561032182296, blk_pad = {13306488860543257747, 13233650979267326550}, blk_phys_birth = 15000687837011192078, blk_birth = 9654056174499278878, 
        blk_fill = 4581854547837491919, blk_cksum = {zc_word = {473834567897273522, 11045675235573507751, 17309151065454408147, 9291837877155058097}}}, {blk_dva = {{dva_word = {17670586488979368792, 
              9287973736594532127}}, {dva_word = {7478846183919920266, 13033103807999935336}}, {dva_word = {873174417743406154, 14033971842230792776}}}, blk_prop = 17602393303572169639, blk_pad = {
          1474845132844031875, 3635131447487278299}, blk_phys_birth = 1908729052115667154, blk_birth = 3050245192693081646, blk_fill = 9369774182031479418, blk_cksum = {zc_word = {9265944745165672062, 
            8501086784592005320, 1489125699408055099, 3411239927155586007}}}}, {__dn_ignore1 = {blk_dva = {{dva_word = {15565337834032293466, 3011959817867889727}}, {dva_word = {10800861183152009571, 
              13580592370867391509}}, {dva_word = {17819155297566903882, 2198318479993026249}}}, blk_prop = 9003424388140510925, blk_pad = {4684284040627411823, 9435362951285916429}, 
        blk_phys_birth = 7315724984906794774, blk_birth = 14284891517734646008, blk_fill = 5789291424850077640, blk_cksum = {zc_word = {9939596259341780421, 10239938790466264068, 12660054648441753599, 
            4953639511686572620}}}, 
      dn_bonus = "\272\267@\202uǟ<\220\364\364_E\370\231\062?h\002\322;\336\070\364,\354\271\025\245ǹå.\313\322\351mQ\374!\210-\274\275\223\247\063\030*3?\225\203\017\215\223\214_,\036%\252\270V\262\272ds_\247\267\016)\223W\034&-\320\036\060\262\372\360\030\372\205Ϛ\\\200o\004\226?\262T\321m\af\223\006\247&\260\260\032\037J\231\323\361\373\253/u6\360\261\321X\252Q=\363\200X\253n\370Ɉ:\365\037\347\337T\347\202倊$U\312\372\062\312gh\203\240l\334\342\336\264J䍴\201#\036\fH2\000\f\t\257\302§C\"\270rCH\364\203\337\341\221ճw\024۬T\257ʔr2҈\322\303\365*}\032.fP\204ץT*"...}, {__dn_ignore2 = {blk_dva = {{dva_word = {
              15565337834032293466, 3011959817867889727}}, {dva_word = {10800861183152009571, 13580592370867391509}}, {dva_word = {17819155297566903882, 2198318479993026249}}}, blk_prop = 9003424388140510925, 
        blk_pad = {4684284040627411823, 9435362951285916429}, blk_phys_birth = 7315724984906794774, blk_birth = 14284891517734646008, blk_fill = 5789291424850077640, blk_cksum = {zc_word = {
            9939596259341780421, 10239938790466264068, 12660054648441753599, 4953639511686572620}}}, 
      __dn_ignore3 = "\272\267@\202uǟ<\220\364\364_E\370\231\062?h\002\322;\336\070\364,\354\271\025\245ǹå.\313\322\351mQ\374!\210-\274\275\223\247\063\030*3?\225\203\017\215\223\214_,\036%\252\270V\262\272ds_\247\267\016)\223W\034&-\320\036\060\262\372\360\030\372\205Ϛ\\\200o\004\226?\262T\321m\af\223\006\247&\260\260\032\037J\231\323\361\373\253/u6\360\261\321X\252Q=\363\200X\253n\370Ɉ:\365\037\347\337T\347\202倊$U\312\372\062\312gh\203\240l\334\342\336\264J䍴\201#\036\fH2\000\f\t\257\302§C\"\270rCH\364\203\337\341\221ճw\024", dn_spill = {blk_dva = {{dva_word = {3635131447487278299, 1908729052115667154}}, {
            dva_word = {3050245192693081646, 9369774182031479418}}, {dva_word = {9265944745165672062, 8501086784592005320}}}, blk_prop = 1489125699408055099, blk_pad = {3411239927155586007, 
          11713873272415966912}, blk_phys_birth = 13484792739961904142, blk_birth = 13265830906695429918, blk_fill = 1686461937261552921, blk_cksum = {zc_word = {8685750855827605437, 16734380903908582676, 
            15410663462902461972, 12462847127971650248}}}}}}

How Has This Been Tested?

Tested manually on local Debian builder, without patch:

root@linux:/usr/src/zfs# POOLNAME='testpool'
root@linux:/usr/src/zfs# TMPDIR='/var/tmp'
root@linux:/usr/src/zfs# mountpoint -q $TMPDIR || mount -t tmpfs tmpfs $TMPDIR
root@linux:/usr/src/zfs# zpool destroy -f $POOLNAME
root@linux:/usr/src/zfs# rm -f $TMPDIR/zpool.dat
root@linux:/usr/src/zfs# truncate -s 128m $TMPDIR/zpool.dat
root@linux:/usr/src/zfs# printf 12345678 > /tmp/zfskey
root@linux:/usr/src/zfs# zpool create -o ashift=12 \
>                 -O acltype=posixacl -O canmount=off -O compression=lz4 \
>                 -O dnodesize=1k -O normalization=formD -O relatime=on -O xattr=sa \
>                 -O encryption=aes-256-gcm -O keylocation=file:///tmp/zfskey -O keyformat=passphrase $POOLNAME $TMPDIR/zpool.dat
root@linux:/usr/src/zfs# 
root@linux:/usr/src/zfs# zfs create $POOLNAME/test -o mountpoint=/mnt/test
root@linux:/usr/src/zfs# zfs snap $POOLNAME/test@create
root@linux:/usr/src/zfs# for i in {1..2}; do for j in {1..2}; do echo $i $j > /mnt/test/${i}.${j}.txt; done; done
root@linux:/usr/src/zfs# zfs diff $POOLNAME/test@create
+	/mnt/test/1.1.txt
Unable to determine path or stats for object 3 in testpool/test@zfs-diff-1359-0000000100019fe9: File exists

With patch applied:

root@linux:~# POOLNAME='testpool'
root@linux:~# TMPDIR='/var/tmp'
root@linux:~# mountpoint -q $TMPDIR || mount -t tmpfs tmpfs $TMPDIR
root@linux:~# zpool destroy -f $POOLNAME
root@linux:~# rm -f $TMPDIR/zpool.dat
root@linux:~# truncate -s 128m $TMPDIR/zpool.dat
root@linux:~# printf 12345678 > /tmp/zfskey
root@linux:~# zpool create -o ashift=12 \
>                 -O acltype=posixacl -O canmount=off -O compression=lz4 \
>                 -O dnodesize=1k -O normalization=formD -O relatime=on -O xattr=sa \
>                 -O encryption=aes-256-gcm -O keylocation=file:///tmp/zfskey -O keyformat=passphrase $POOLNAME $TMPDIR/zpool.dat
root@linux:~# 
root@linux:~# zfs create $POOLNAME/test -o mountpoint=/mnt/test
root@linux:~# zfs snap $POOLNAME/test@create
root@linux:~# for i in {1..2}; do for j in {1..2}; do echo $i $j > /mnt/test/${i}.${j}.txt; done; done
root@linux:~# zfs diff $POOLNAME/test@create
+	/mnt/test/1.1.txt
+	/mnt/test/1.2.txt
+	/mnt/test/2.1.txt
+	/mnt/test/2.2.txt
M	/mnt/test/
root@linux:~# 
root@linux:~# ls -li /mnt/test/1.2.txt
4 -rw-r--r-- 1 root root 4 Sep 22 09:27 /mnt/test/1.2.txt
root@linux:~# zdb -d $POOLNAME/test 3
Dataset testpool/test [ZPL], ID 138, cr_txg 6, 208K, 10 objects

    Object  lvl   iblk   dblk  dsize  dnsize  lsize   %full  type
zdb: dmu_object_info() failed, errno 17
root@linux:~#

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.

loli10K · 2019-09-22T09:58:34Z

Same issue without encryption but heavy usage of SA based xattrs, this commit will probably fix #7678 too.

root@linux:~# POOLNAME='testpool'
root@linux:~# TMPDIR='/var/tmp'
root@linux:~# mountpoint -q $TMPDIR || mount -t tmpfs tmpfs $TMPDIR
root@linux:~# zpool destroy -f $POOLNAME
root@linux:~# rm -f $TMPDIR/zpool.dat
root@linux:~# truncate -s 128m $TMPDIR/zpool.dat
root@linux:~# zpool create -O dnodesize=1k -O xattr=sa -O mountpoint=none $POOLNAME $TMPDIR/zpool.dat
root@linux:~# zfs create $POOLNAME/test -o mountpoint=/mnt/test
root@linux:~# zfs snap $POOLNAME/test@create
root@linux:~# for i in {1..2}; do for j in {1..2}; do echo $i $j > /mnt/test/${i}.${j}.txt; done; done
root@linux:~# zfs diff $POOLNAME/test@create
+	/mnt/test/1.1.txt
+	/mnt/test/1.2.txt
+	/mnt/test/2.1.txt
+	/mnt/test/2.2.txt
M	/mnt/test/
root@linux:~# for i in {1..100}; do
>   setfattr -n user.value$i -v $(python -c 'print "A"*128') /mnt/test/1.1.txt
> done
root@linux:~# zfs diff $POOLNAME/test@create
+	/mnt/test/1.1.txt
Unable to determine path or stats for object 3 in testpool/test@zfs-diff-1861-00000000ffff2fde: File exists
root@linux:~#

Thread 187 hit Breakpoint 1, report_dnode (da=0xffff8800b8757d60, object=3, dnp=0xffff8800b8cc4600) at /usr/src/zfs/module/zfs/dmu_diff.c:86
86	{
(gdb) up
#1  0xffffffffc0231355 in diff_cb (spa=0xffff8800b86f8000, zilog=0x0 <irq_stack_union>, bp=0xffffc90000e53000, zb=0xffff8800b8693740, dnp=0xffff8800b8cbf000, arg=0xffff8800b8757d60)
    at /usr/src/zfs/module/zfs/dmu_diff.c:149
149				err = report_dnode(da, dnobj, blk+i);
(gdb) x/64x blk+2
0xffff8800b8cc4400:	0x01011113	0x0f00002c	0x02c00001	0x00000001
0xffff8800b8cc4410:	0x00000000	0x00000000	0x00000a00	0x00000000
0xffff8800b8cc4420:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b8cc4430:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b8cc4440:	0x00000001	0x00000000	0x000000d7	0x00000000
0xffff8800b8cc4450:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b8cc4460:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b8cc4470:	0x00000000	0x80130702	0x00000000	0x00000000
0xffff8800b8cc4480:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b8cc4490:	0x00000009	0x00000000	0x00000001	0x00000000
0xffff8800b8cc44a0:	0x0a312031	0x00000000	0x18901880	0x00000005
0xffff8800b8cc44b0:	0xb04e2c40	0x00000148	0x2d3b7d80	0x000037a3
0xffff8800b8cc44c0:	0x002f505a	0x00180402	0x000081a4	0x00000000
0xffff8800b8cc44d0:	0x00000004	0x00000000	0x00000009	0x00000000
0xffff8800b8cc44e0:	0x00000000	0x00000000	0x00000000	0x00000000
0xffff8800b8cc44f0:	0x00000022	0x00000000	0x00000004	0x00008408
(gdb) x/64x blk+3
0xffff8800b8cc4600:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc4610:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc4620:	0x41414141	0xa0000000	0xa0000000	0x0b000000
0xffff8800b8cc4630:	0x72657375	0x6c61762e	0x00326575	0x0a000000
0xffff8800b8cc4640:	0x80000000	0x41414141	0x41414141	0x41414141
0xffff8800b8cc4650:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc4660:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc4670:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc4680:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc4690:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc46a0:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc46b0:	0x41414141	0x41414141	0x41414141	0x41414141
0xffff8800b8cc46c0:	0x41414141	0xa0000000	0xa0000000	0x0b000000
0xffff8800b8cc46d0:	0x72657375	0x6c61762e	0x00336575	0x0a000000
0xffff8800b8cc46e0:	0x80000000	0x41414141	0x41414141	0x41414141
0xffff8800b8cc46f0:	0x41414141	0x41414141	0x41414141	0x41414141
(gdb) p *(blk+3)
$1 = {dn_type = 65 'A', dn_indblkshift = 65 'A', dn_nlevels = 65 'A', dn_nblkptr = 65 'A', dn_bonustype = 65 'A', dn_checksum = 65 'A', dn_compress = 65 'A', dn_flags = 65 'A', dn_datablkszsec = 16705, 
  dn_bonuslen = 16705, dn_extra_slots = 65 'A', dn_pad2 = "AAA", dn_maxblkid = 4702111234474983745, dn_used = 4702111234474983745, dn_pad3 = {11529215047163265345, 792633537101561856, 7809653170696975221, 
    720575940382582133}, {dn_blkptr = {{blk_dva = {{dva_word = {4702111235527671808, 4702111234474983745}}, {dva_word = {4702111234474983745, 4702111234474983745}}, {dva_word = {4702111234474983745, 
              4702111234474983745}}}, blk_prop = 4702111234474983745, blk_pad = {4702111234474983745, 4702111234474983745}, blk_phys_birth = 4702111234474983745, blk_birth = 4702111234474983745, 
        blk_fill = 4702111234474983745, blk_cksum = {zc_word = {4702111234474983745, 4702111234474983745, 4702111234474983745, 4702111234474983745}}}, {blk_dva = {{dva_word = {11529215047163265345, 
              792633537101561856}}, {dva_word = {7809653170696975221, 720575940382647669}}, {dva_word = {4702111235527671808, 4702111234474983745}}}, blk_prop = 4702111234474983745, blk_pad = {
          4702111234474983745, 4702111234474983745}, blk_phys_birth = 4702111234474983745, blk_birth = 4702111234474983745, blk_fill = 4702111234474983745, blk_cksum = {zc_word = {4702111234474983745, 
            4702111234474983745, 4702111234474983745, 4702111234474983745}}}, {blk_dva = {{dva_word = {4702111234474983745, 4702111234474983745}}, {dva_word = {4702111234474983745, 4702111234474983745}}, {
            dva_word = {1094795585, 0}}}, blk_prop = 0, blk_pad = {0, 2}, blk_phys_birth = 312, blk_birth = 2, blk_fill = 33076, blk_cksum = {zc_word = {0, 0, 9235764696836014111, 0}}}}, {__dn_ignore1 = {
        blk_dva = {{dva_word = {4702111235527671808, 4702111234474983745}}, {dva_word = {4702111234474983745, 4702111234474983745}}, {dva_word = {4702111234474983745, 4702111234474983745}}}, 
        blk_prop = 4702111234474983745, blk_pad = {4702111234474983745, 4702111234474983745}, blk_phys_birth = 4702111234474983745, blk_birth = 4702111234474983745, blk_fill = 4702111234474983745, blk_cksum = {
          zc_word = {4702111234474983745, 4702111234474983745, 4702111234474983745, 4702111234474983745}}}, 
      dn_bonus = "AAAA\000\000\000\240\000\000\000\240\000\000\000\vuser.value3\000\000\000\000\n\000\000\000\200", 'A' <repeats 128 times>, '\000' <repeats 28 times>, "\002\000\000\000\000\000\000\000"...}, {
      __dn_ignore2 = {blk_dva = {{dva_word = {4702111235527671808, 4702111234474983745}}, {dva_word = {4702111234474983745, 4702111234474983745}}, {dva_word = {4702111234474983745, 4702111234474983745}}}, 
        blk_prop = 4702111234474983745, blk_pad = {4702111234474983745, 4702111234474983745}, blk_phys_birth = 4702111234474983745, blk_birth = 4702111234474983745, blk_fill = 4702111234474983745, blk_cksum = {
          zc_word = {4702111234474983745, 4702111234474983745, 4702111234474983745, 4702111234474983745}}}, 
      __dn_ignore3 = "AAAA\000\000\000\240\000\000\000\240\000\000\000\vuser.value3\000\000\000\000\n\000\000\000\200", 'A' <repeats 128 times>, '\000' <repeats 27 times>, dn_spill = {blk_dva = {{dva_word = {
              2, 312}}, {dva_word = {2, 33076}}, {dva_word = {0, 0}}}, blk_prop = 9235764696836014111, blk_pad = {0, 0}, blk_phys_birth = 0, blk_birth = 11, blk_fill = 1, blk_cksum = {zc_word = {151737245388, 
            27743384833715, 2688993025771289, 182578471246767068}}}}}}

EDIT: confirmed reproducer on 0.7.13, this commit fixes the issue.

behlendorf

Makes sense, thanks for getting to the bottom of this!

loli10K · 2019-09-23T06:16:22Z

Pushed a small update to the ZTS script (forgot to destroy the second dataset in cleanup()) and reworded the commit message to mention SA based xattrs

codecov · 2019-09-23T07:34:50Z

Codecov Report

Merging #9343 into master will increase coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #9343     +/-   ##
=========================================
+ Coverage   79.06%   79.16%   +0.1%     
=========================================
  Files         401      401             
  Lines      122495   122494      -1     
=========================================
+ Hits        96846    96971    +125     
+ Misses      25649    25523    -126

Flag	Coverage Δ
#kernel	`79.85% <100%> (+0.09%)`	⬆️
#user	`66.7% <0%> (+0.07%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73d7820...069fbef. Read the comment docs.

module/zfs/dmu_diff.c

tcaputi

LGTM

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. Signed-off-by: loli10K <[email protected]>

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 <[email protected]> Reviewed-by: Tom Caputi <[email protected]> Reviewed-by: Ryan Moeller <[email protected]> Signed-off-by: loli10K <[email protected]> Closes openzfs#7678 Closes openzfs#8931 Closes openzfs#9343

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 <[email protected]> Reviewed-by: Tom Caputi <[email protected]> Reviewed-by: Ryan Moeller <[email protected]> Signed-off-by: loli10K <[email protected]> Closes #7678 Closes #8931 Closes #9343

loli10K added the Status: Code Review Needed Ready for review and testing label Sep 22, 2019

loli10K force-pushed the issue-8931 branch from a31e940 to 60954a3 Compare September 22, 2019 08:11

behlendorf requested a review from tcaputi September 22, 2019 22:19

behlendorf approved these changes Sep 22, 2019

View reviewed changes

loli10K force-pushed the issue-8931 branch 2 times, most recently from 848ec88 to 31acea0 Compare September 23, 2019 06:16

ghost reviewed Sep 23, 2019

View reviewed changes

module/zfs/dmu_diff.c Outdated Show resolved Hide resolved

tcaputi approved these changes Sep 23, 2019

View reviewed changes

loli10K force-pushed the issue-8931 branch from 31acea0 to 069fbef Compare September 23, 2019 17:59

ghost approved these changes Sep 23, 2019

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 23, 2019

behlendorf merged commit d359e99 into openzfs:master Sep 24, 2019

behlendorf mentioned this pull request Dec 3, 2019

"zfs diff" is unable to determine path or stats for an object #948

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

diff_cb() does not handle encrypted large dnodes #9343

diff_cb() does not handle encrypted large dnodes #9343

loli10K commented Sep 22, 2019 •

edited

Loading

loli10K commented Sep 22, 2019 •

edited

Loading

behlendorf left a comment

loli10K commented Sep 23, 2019

codecov bot commented Sep 23, 2019 •

edited

Loading

tcaputi left a comment

diff_cb() does not handle encrypted large dnodes #9343

diff_cb() does not handle encrypted large dnodes #9343

Conversation

loli10K commented Sep 22, 2019 • edited Loading

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

loli10K commented Sep 22, 2019 • edited Loading

behlendorf left a comment

Choose a reason for hiding this comment

loli10K commented Sep 23, 2019

codecov bot commented Sep 23, 2019 • edited Loading

Codecov Report

tcaputi left a comment

Choose a reason for hiding this comment

loli10K commented Sep 22, 2019 •

edited

Loading

loli10K commented Sep 22, 2019 •

edited

Loading

codecov bot commented Sep 23, 2019 •

edited

Loading