Avoid panic with recordsize > 128k, raw sending and no large_blocks

The current codebase does not support raw sending buffers with block size > 128kB when large_blocks is not active. This can happen in the codepath dsl_dataset_sync()->dmu_objset_sync()->zio_nowait() which calls back dmu_objset_write_done()->dsl_dataset_block_born(). If dsl_dataset_sync() completes its run before dsl_dataset_block_born() is called, we will end up not activating some of the necessary flags, while having blocks based on those flags written in the filesystem. A subsequent send will then panic. Fix this by directly deciding in dmu_objset_sync() whether these flags need to be activated later by dsl_dataset_sync(). Instead of panicking due to a NULL pointer dereference in dmu_dump_write() in case of a send, print out an error message. Also during scrub verify there are no contradicting filesystem flags. Reviewed-by: Paul Dagnelie <[email protected]> Signed-off-by: George Amanakis <[email protected]> Closes #12275 Closes #12438
openzfs · Jun 27, 2022 · 80a650b · 80a650b
1 parent 1cd72b9
commit 80a650b
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 20 deletions.
diff --git a/include/sys/dsl_dataset.h b/include/sys/dsl_dataset.h
@@ -373,6 +373,7 @@ boolean_t dsl_dataset_modified_since_snap(dsl_dataset_t *ds,
 void dsl_dataset_sync(dsl_dataset_t *ds, zio_t *zio, dmu_tx_t *tx);
 void dsl_dataset_sync_done(dsl_dataset_t *ds, dmu_tx_t *tx);
 
+void dsl_dataset_feature_set_activation(const blkptr_t *bp, dsl_dataset_t *ds);
 void dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp,
     dmu_tx_t *tx);
 int dsl_dataset_block_kill(dsl_dataset_t *ds, const blkptr_t *bp,

diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c
@@ -851,6 +851,11 @@ dump_ioctl(zfs_handle_t *zhp, const char *fromsnap, uint64_t fromsnap_obj,
 		case EINVAL:
 			zfs_error_aux(hdl, "%s", strerror(errno));
 			return (zfs_error(hdl, EZFS_BADBACKUP, errbuf));
+		case ENOTSUP:
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "large blocks detected but large_blocks feature "
+			    "is inactive; raw send unsupported"));
+			return (zfs_error(hdl, EZFS_NOTSUP, errbuf));
 
 		default:
 			return (zfs_standard_error(hdl, errno, errbuf));
@@ -2674,6 +2679,11 @@ zfs_send_one_cb_impl(zfs_handle_t *zhp, const char *from, int fd,
 		case EROFS:
 			zfs_error_aux(hdl, "%s", strerror(errno));
 			return (zfs_error(hdl, EZFS_BADBACKUP, errbuf));
+		case ENOTSUP:
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "large blocks detected but large_blocks feature "
+			    "is inactive; raw send unsupported"));
+			return (zfs_error(hdl, EZFS_NOTSUP, errbuf));
 
 		default:
 			return (zfs_standard_error(hdl, errno, errbuf));

diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c
@@ -1695,6 +1695,16 @@ dmu_objset_sync(objset_t *os, zio_t *pio, dmu_tx_t *tx)
 	    &zp, dmu_objset_write_ready, NULL, NULL, dmu_objset_write_done,
 	    os, ZIO_PRIORITY_ASYNC_WRITE, ZIO_FLAG_MUSTSUCCEED, &zb);
 
+	/*
+	 * In the codepath dsl_dataset_sync()->dmu_objset_sync() we cannot
+	 * rely on the zio above completing and calling back
+	 * dmu_objset_write_done()->dsl_dataset_block_born() before
+	 * dsl_dataset_sync() actually activates feature flags near its end.
+	 * Decide here if any features need to be activated, before
+	 * dsl_dataset_sync() completes its run.
+	 */
+	dsl_dataset_feature_set_activation(blkptr_copy, os->os_dsl_dataset);
+
 	/*
 	 * Sync special dnodes - the parent IO for the sync is the root block
 	 */

diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c
@@ -493,6 +493,7 @@ dmu_dump_write(dmu_send_cookie_t *dscp, dmu_object_type_t type, uint64_t object,
 	    (bp != NULL ? BP_GET_COMPRESS(bp) != ZIO_COMPRESS_OFF &&
 	    io_compressed : lsize != psize);
 	if (raw || compressed) {
+		ASSERT(bp != NULL);
 		ASSERT(raw || dscp->dsc_featureflags &
 		    DMU_BACKUP_FEATURE_COMPRESSED);
 		ASSERT(!BP_IS_EMBEDDED(bp));
@@ -1017,6 +1018,9 @@ do_dump(dmu_send_cookie_t *dscp, struct send_range *range)
 		if (srdp->datablksz > SPA_OLD_MAXBLOCKSIZE &&
 		    !(dscp->dsc_featureflags &
 		    DMU_BACKUP_FEATURE_LARGE_BLOCKS)) {
+			if (dscp->dsc_featureflags & DMU_BACKUP_FEATURE_RAW)
+				return (SET_ERROR(ENOTSUP));
+
 			while (srdp->datablksz > 0 && err == 0) {
 				int n = MIN(srdp->datablksz,
 				    SPA_OLD_MAXBLOCKSIZE);

diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c
@@ -130,6 +130,30 @@ parent_delta(dsl_dataset_t *ds, int64_t delta)
 	return (new_bytes - old_bytes);
 }
 
+void
+dsl_dataset_feature_set_activation(const blkptr_t *bp, dsl_dataset_t *ds)
+{
+	spa_feature_t f;
+	if (BP_GET_LSIZE(bp) > SPA_OLD_MAXBLOCKSIZE) {
+		ds->ds_feature_activation[SPA_FEATURE_LARGE_BLOCKS] =
+		    (void *)B_TRUE;
+	}
+
+	f = zio_checksum_to_feature(BP_GET_CHECKSUM(bp));
+	if (f != SPA_FEATURE_NONE) {
+		ASSERT3S(spa_feature_table[f].fi_type, ==,
+		    ZFEATURE_TYPE_BOOLEAN);
+		ds->ds_feature_activation[f] = (void *)B_TRUE;
+	}
+
+	f = zio_compress_to_feature(BP_GET_COMPRESS(bp));
+	if (f != SPA_FEATURE_NONE) {
+		ASSERT3S(spa_feature_table[f].fi_type, ==,
+		    ZFEATURE_TYPE_BOOLEAN);
+		ds->ds_feature_activation[f] = (void *)B_TRUE;
+	}
+}
+
 void
 dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx)
 {
@@ -138,7 +162,6 @@ dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx)
 	int compressed = BP_GET_PSIZE(bp);
 	int uncompressed = BP_GET_UCSIZE(bp);
 	int64_t delta;
-	spa_feature_t f;
 
 	dprintf_bp(bp, "ds=%p", ds);
 
@@ -163,25 +186,7 @@ dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx)
 	dsl_dataset_phys(ds)->ds_uncompressed_bytes += uncompressed;
 	dsl_dataset_phys(ds)->ds_unique_bytes += used;
 
-	if (BP_GET_LSIZE(bp) > SPA_OLD_MAXBLOCKSIZE) {
-		ds->ds_feature_activation[SPA_FEATURE_LARGE_BLOCKS] =
-		    (void *)B_TRUE;
-	}
-
-
-	f = zio_checksum_to_feature(BP_GET_CHECKSUM(bp));
-	if (f != SPA_FEATURE_NONE) {
-		ASSERT3S(spa_feature_table[f].fi_type, ==,
-		    ZFEATURE_TYPE_BOOLEAN);
-		ds->ds_feature_activation[f] = (void *)B_TRUE;
-	}
-
-	f = zio_compress_to_feature(BP_GET_COMPRESS(bp));
-	if (f != SPA_FEATURE_NONE) {
-		ASSERT3S(spa_feature_table[f].fi_type, ==,
-		    ZFEATURE_TYPE_BOOLEAN);
-		ds->ds_feature_activation[f] = (void *)B_TRUE;
-	}
+	dsl_dataset_feature_set_activation(bp, ds);
 
 	/*
 	 * Track block for livelist, but ignore embedded blocks because
@@ -5013,3 +5018,4 @@ EXPORT_SYMBOL(dsl_dsobj_to_dsname);
 EXPORT_SYMBOL(dsl_dataset_check_quota);
 EXPORT_SYMBOL(dsl_dataset_clone_swap_check_impl);
 EXPORT_SYMBOL(dsl_dataset_clone_swap_sync_impl);
+EXPORT_SYMBOL(dsl_dataset_feature_set_activation);
diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c
@@ -2004,6 +2004,21 @@ dsl_scan_visitbp(blkptr_t *bp, const zbookmark_phys_t *zb,
 		return;
 	}
 
+	/*
+	 * Check if this block contradicts any filesystem flags.
+	 */
+	spa_feature_t f = SPA_FEATURE_LARGE_BLOCKS;
+	if (BP_GET_LSIZE(bp) > SPA_OLD_MAXBLOCKSIZE)
+		ASSERT3B(dsl_dataset_feature_is_active(ds, f), ==, B_TRUE);
+
+	f = zio_checksum_to_feature(BP_GET_CHECKSUM(bp));
+	if (f != SPA_FEATURE_NONE)
+		ASSERT3B(dsl_dataset_feature_is_active(ds, f), ==, B_TRUE);
+
+	f = zio_compress_to_feature(BP_GET_COMPRESS(bp));
+	if (f != SPA_FEATURE_NONE)
+		ASSERT3B(dsl_dataset_feature_is_active(ds, f), ==, B_TRUE);
+
 	if (bp->blk_birth <= scn->scn_phys.scn_cur_min_txg) {
 		scn->scn_lt_min_this_txg++;
 		return;