From 0433523ca2632a175546e0cc99d1812de2a90d55 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 13 Mar 2025 11:07:48 -0600 Subject: [PATCH] Verify every block pointer is either embedded, hole, or has a valid DVA Now instead of crashing when attempting to read the corrupt block pointer, ZFS will return ECKSUM, in a stack that looks like this: ``` none:set-error zfs.ko`arc_read+0x1d82 zfs.ko`dbuf_read+0xa8c zfs.ko`dmu_buf_hold_array_by_dnode+0x292 zfs.ko`dmu_read_uio_dnode+0x47 zfs.ko`zfs_read+0x2d5 zfs.ko`zfs_freebsd_read+0x7b kernel`VOP_READ_APV+0xd0 kernel`vn_read+0x20e kernel`vn_io_fault_doio+0x45 kernel`vn_io_fault1+0x15e kernel`vn_io_fault+0x150 kernel`dofileread+0x80 kernel`sys_read+0xb7 kernel`amd64_syscall+0x424 kernel`0xffffffff810633cb ``` This patch should hopefully also prevent such corrupt block pointers from being written to disk in the first place. And in zdb, don't crash when printing a block pointer with no valid DVAs. If a block pointer isn't embedded yet doesn't have any valid DVAs, that's a data corruption bug. zdb should be able to handle the situation gracefully. Finally, remove an extra check for gang blocks in SNPRINTF_BLKPTR. This check, which compares the asizes of two different DVAs within the same BP, was added by illumos-gate commit b24ab67[^1], and I can't understand why. It doesn't appear to do anything useful, so remove it. [^1]: https://github.com/illumos/illumos-gate/commit/b24ab6762772a3f6a89393947930c7fa61306783 Fixes #17077 Sponsored by: ConnectWise Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Reviewed by: Alek Pinchuk Signed-off-by: Alan Somers Closes #17078 --- include/sys/spa.h | 5 ----- module/zfs/zio.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/include/sys/spa.h b/include/sys/spa.h index 715be9eb1..1606d45e2 100644 --- a/include/sys/spa.h +++ b/include/sys/spa.h @@ -670,7 +670,6 @@ typedef struct blkptr { (u_longlong_t)DVA_GET_ASIZE(dva), \ ws); \ } \ - ASSERT3S(copies, >, 0); \ if (BP_IS_ENCRYPTED(bp)) { \ len += func(buf + len, size - len, \ "salt=%llx iv=%llx:%llx%c", \ @@ -679,10 +678,6 @@ typedef struct blkptr { (u_longlong_t)BP_GET_IV2(bp), \ ws); \ } \ - if (BP_IS_GANG(bp) && \ - DVA_GET_ASIZE(&bp->blk_dva[2]) <= \ - DVA_GET_ASIZE(&bp->blk_dva[1]) / 2) \ - copies--; \ len += func(buf + len, size - len, \ "[L%llu %s] %s %s %s %s %s %s %s%c" \ "size=%llxL/%llxP birth=%lluL/%lluP fill=%llu%c" \ diff --git a/module/zfs/zio.c b/module/zfs/zio.c index 36e2f5e4b..9058c1908 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -1212,6 +1212,16 @@ zfs_blkptr_verify(spa_t *spa, const blkptr_t *bp, bp, (longlong_t)BPE_GET_PSIZE(bp)); } return (errors ? ECKSUM : 0); + } else if (BP_IS_HOLE(bp)) { + /* + * Holes are allowed (expected, even) to have no DVAs, no + * checksum, and no psize. + */ + return (errors ? ECKSUM : 0); + } else if (unlikely(!DVA_IS_VALID(&bp->blk_dva[0]))) { + /* Non-hole, non-embedded BPs _must_ have at least one DVA */ + errors += zfs_blkptr_verify_log(spa, bp, blk_verify, + "blkptr at %px has no valid DVAs", bp); } if (unlikely(BP_GET_CHECKSUM(bp) >= ZIO_CHECKSUM_FUNCTIONS)) { errors += zfs_blkptr_verify_log(spa, bp, blk_verify,