From f7b32324c6507551264f667044111cb6c53fac58 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 20 Feb 2025 11:07:57 -0700 Subject: [PATCH 1/4] 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. Issue #17077 Sponsored by: ConnectWise Signed-off-by: Alan Somers --- include/sys/spa.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/sys/spa.h b/include/sys/spa.h index 23c20294d1f8..2e5bb0f485e0 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", \ @@ -695,7 +694,8 @@ typedef struct blkptr { BP_GET_BYTEORDER(bp) == 0 ? "BE" : "LE", \ BP_IS_GANG(bp) ? "gang" : "contiguous", \ BP_GET_DEDUP(bp) ? "dedup" : "unique", \ - copyname[copies], \ + copies >= 0 && copies <= 3 ? copyname[copies] : \ + "invalid", \ ws, \ (u_longlong_t)BP_GET_LSIZE(bp), \ (u_longlong_t)BP_GET_PSIZE(bp), \ From 00fb087019a0a9b7e526bc868228f9fc02f6c9c1 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 20 Feb 2025 17:11:58 +0000 Subject: [PATCH 2/4] 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. Fixes #17077 Sponsored by: ConnectWise Signed-off-by: Alan Somers --- module/zfs/zio.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/module/zfs/zio.c b/module/zfs/zio.c index b071ac17ed1f..c39e863ba910 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -1213,6 +1213,11 @@ zfs_blkptr_verify(spa_t *spa, const blkptr_t *bp, } return (errors == 0); } + if (unlikely(!BP_IS_EMBEDDED(bp) && !BP_IS_HOLE(bp) && + BP_GET_NDVAS(bp) <= 0)) { + 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, "blkptr at %px has invalid CHECKSUM %llu", From 19fe2e0c6f3ad07fdbe5b6b6e03b72c8fb865188 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 21 Feb 2025 09:13:25 -0700 Subject: [PATCH 3/4] Tighten the DVA validity check, as requested by @amotin Every non-embedded, non-hole BP should have at least one valid DVA, and it must be the first one. Signed-off-by: Alan Somers --- module/zfs/zio.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/module/zfs/zio.c b/module/zfs/zio.c index c39e863ba910..f2f21b4532b7 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -1212,9 +1212,10 @@ zfs_blkptr_verify(spa_t *spa, const blkptr_t *bp, bp, (longlong_t)BPE_GET_PSIZE(bp)); } return (errors == 0); - } - if (unlikely(!BP_IS_EMBEDDED(bp) && !BP_IS_HOLE(bp) && - BP_GET_NDVAS(bp) <= 0)) { + } else if (BP_IS_HOLE(bp)) { + /* Holes are allowed (expected, even) to have no DVAs */ + } 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); } From 5ccd822d3e5f1035e3746ab501a8c032e873e1db Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 21 Feb 2025 10:32:55 -0700 Subject: [PATCH 4/4] Return early when verifying hole blkptrs This saves us the trouble of acquring the spa config lock, only to do nothing with it. Signed-off-by: Alan Somers --- module/zfs/zio.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/module/zfs/zio.c b/module/zfs/zio.c index f2f21b4532b7..dcc74dbfc65d 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -1213,7 +1213,11 @@ zfs_blkptr_verify(spa_t *spa, const blkptr_t *bp, } return (errors == 0); } else if (BP_IS_HOLE(bp)) { - /* Holes are allowed (expected, even) to have no DVAs */ + /* + * Holes are allowed (expected, even) to have no DVAs, no + * checksum, and no psize. + */ + return (errors == 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,