Send / Recv Fixes following b52563

This patch fixes several issues discovered after
the encryption patch was merged:

* Fixed a bug where encrypted datasets could attempt
  to receive embedded data records.

* Fixed a bug where dirty records created by the recv
  code wasn't properly setting the dr_raw flag.

* Fixed a typo where a dmu_tx_commit() was changed to
  dmu_tx_abort()

* Fixed a few error handling bugs unrelated to the
  encryption patch in dmu_recv_stream()

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes #6512 
Closes #6524 
Closes #6545 

6 files changed, 88 insertions, 48 deletions

diff --git a/include/sys/dmu.h b/include/sys/dmu.h
index 7c7e6dcbf..60778289e 100644
--- a/include/sys/dmu.h
+++ b/include/sys/dmu.h
@@ -448,8 +448,9 @@ void dmu_object_set_checksum(objset_t *os, uint64_t object, uint8_t checksum,
 void dmu_object_set_compress(objset_t *os, uint64_t object, uint8_t compress,
     dmu_tx_t *tx);
 
-void
-dmu_write_embedded(objset_t *os, uint64_t object, uint64_t offset,
+int dmu_object_dirty_raw(objset_t *os, uint64_t object, dmu_tx_t *tx);
+
+void dmu_write_embedded(objset_t *os, uint64_t object, uint64_t offset,
     void *data, uint8_t etype, uint8_t comp, int uncompressed_size,
     int compressed_size, int byteorder, dmu_tx_t *tx);
 
diff --git a/include/sys/zfs_ioctl.h b/include/sys/zfs_ioctl.h
index 904588271..6924280c4 100644
--- a/include/sys/zfs_ioctl.h
+++ b/include/sys/zfs_ioctl.h
@@ -164,11 +164,9 @@ typedef enum dmu_send_resume_token_version {
  * DRR_WRITE_BYREF, and DRR_OBJECT_RANGE blocks
  */
 #define	DRR_CHECKSUM_DEDUP	(1<<0) /* not used for DRR_SPILL blocks */
-#define	DRR_RAW_ENCRYPTED	(1<<1)
-#define	DRR_RAW_BYTESWAP	(1<<2)
+#define	DRR_RAW_BYTESWAP	(1<<1)
 
 #define	DRR_IS_DEDUP_CAPABLE(flags)	((flags) & DRR_CHECKSUM_DEDUP)
-#define	DRR_IS_RAW_ENCRYPTED(flags)	((flags) & DRR_RAW_ENCRYPTED)
 #define	DRR_IS_RAW_BYTESWAPPED(flags)	((flags) & DRR_RAW_BYTESWAP)
 
 /* deal with compressed drr_write replay records */
@@ -177,11 +175,11 @@ typedef enum dmu_send_resume_token_version {
 	(DRR_WRITE_COMPRESSED(drrw) ? (drrw)->drr_compressed_size : \
 	(drrw)->drr_logical_size)
 #define	DRR_SPILL_PAYLOAD_SIZE(drrs) \
-	(DRR_IS_RAW_ENCRYPTED(drrs->drr_flags) ? \
+	((drrs)->drr_compressed_size ? \
 	(drrs)->drr_compressed_size : (drrs)->drr_length)
 #define	DRR_OBJECT_PAYLOAD_SIZE(drro) \
-	(DRR_IS_RAW_ENCRYPTED(drro->drr_flags) ? \
-	drro->drr_raw_bonuslen : P2ROUNDUP(drro->drr_bonuslen, 8))
+	((drro)->drr_raw_bonuslen != 0 ? \
+	(drro)->drr_raw_bonuslen : P2ROUNDUP((drro)->drr_bonuslen, 8))
 
 /*
  * zfs ioctl command structure
@@ -221,7 +219,7 @@ typedef struct dmu_replay_record {
 			uint8_t drr_flags;
 			uint32_t drr_raw_bonuslen;
 			uint64_t drr_toguid;
-			/* only nonzero if DRR_RAW_ENCRYPTED flag is set */
+			/* only nonzero for raw streams */
 			uint8_t drr_indblkshift;
 			uint8_t drr_nlevels;
 			uint8_t drr_nblkptr;
@@ -247,7 +245,7 @@ typedef struct dmu_replay_record {
 			ddt_key_t drr_key;
 			/* only nonzero if drr_compressiontype is not 0 */
 			uint64_t drr_compressed_size;
-			/* only nonzero if DRR_RAW_ENCRYPTED flag is set */
+			/* only nonzero for raw streams */
 			uint8_t drr_salt[ZIO_DATA_SALT_LEN];
 			uint8_t drr_iv[ZIO_DATA_IV_LEN];
 			uint8_t drr_mac[ZIO_DATA_MAC_LEN];
@@ -282,7 +280,7 @@ typedef struct dmu_replay_record {
 			uint8_t drr_flags;
 			uint8_t drr_compressiontype;
 			uint8_t drr_pad[6];
-			/* only nonzero if DRR_RAW_ENCRYPTED flag is set */
+			/* only nonzero for raw streams */
 			uint64_t drr_compressed_size;
 			uint8_t drr_salt[ZIO_DATA_SALT_LEN];
 			uint8_t drr_iv[ZIO_DATA_IV_LEN];
diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c
index c6ad06951..3146b4e61 100644
--- a/lib/libzfs/libzfs_sendrecv.c
+++ b/lib/libzfs/libzfs_sendrecv.c
@@ -3735,6 +3735,8 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
 	    DMU_BACKUP_FEATURE_RESUMING;
 	boolean_t raw = DMU_GET_FEATUREFLAGS(drrb->drr_versioninfo) &
 	    DMU_BACKUP_FEATURE_RAW;
+	boolean_t embedded = DMU_GET_FEATUREFLAGS(drrb->drr_versioninfo) &
+	    DMU_BACKUP_FEATURE_EMBED_DATA;
 	stream_wantsnewfs = (drrb->drr_fromguid == 0 ||
 	    (drrb->drr_flags & DRR_FLAG_CLONE) || originsnap) && !resuming;
 
@@ -4132,6 +4134,10 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
 				    "kernel modules must be upgraded to "
 				    "receive this stream."));
+			if (embedded && !raw)
+				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+				    "incompatible embedded data stream "
+				    "feature with encrypted receive."));
 			(void) zfs_error(hdl, EZFS_BADSTREAM, errbuf);
 			break;
 		case ECKSUM:
diff --git a/man/man8/zfs.8 b/man/man8/zfs.8
index f344eb943..46306e9cc 100644
--- a/man/man8/zfs.8
+++ b/man/man8/zfs.8
@@ -3338,7 +3338,8 @@ feature enabled.
 If the
 .Sy lz4_compress
 feature is active on the sending system, then the receiving system must have
-that feature enabled as well.
+that feature enabled as well. Note that streams generated using this flag are
+unable to be received into an encrypted dataset.
 See
 .Xr zpool-features 5
 for details on ZFS feature flags and the
diff --git a/module/zfs/dmu.c b/module/zfs/dmu.c
index 5b90818f4..1eb35b935 100644
--- a/module/zfs/dmu.c
+++ b/module/zfs/dmu.c
@@ -2016,6 +2016,25 @@ dmu_object_set_compress(objset_t *os, uint64_t object, uint8_t compress,
 	dnode_rele(dn, FTAG);
 }
 
+/*
+ * Dirty an object and set the dirty record's raw flag. This is used
+ * when writing raw data to an object that will not effect the
+ * encryption parameters, specifically during raw receives.
+ */
+int
+dmu_object_dirty_raw(objset_t *os, uint64_t object, dmu_tx_t *tx)
+{
+	dnode_t *dn;
+	int err;
+
+	err = dnode_hold(os, object, FTAG, &dn);
+	if (err)
+		return (err);
+	dmu_buf_will_change_crypt_params((dmu_buf_t *)dn->dn_dbuf, tx);
+	dnode_rele(dn, FTAG);
+	return (err);
+}
+
 int zfs_mdcomp_disable = 0;
 
 /*
diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c
index aca50197b..359ef99d3 100644
--- a/module/zfs/dmu_send.c
+++ b/module/zfs/dmu_send.c
@@ -327,12 +327,11 @@ dump_write(dmu_sendarg_t *dsp, dmu_object_type_t type, uint64_t object,
 			ASSERT(BP_IS_PROTECTED(bp));
 
 			/*
-			 * This is a raw protected block so we set the encrypted
-			 * flag. We need to pass along everything the receiving
-			 * side will need to interpret this block, including the
-			 * byteswap, salt, IV, and MAC.
+			 * This is a raw protected block so we need to pass
+			 * along everything the receiving side will need to
+			 * interpret this block, including the byteswap, salt,
+			 * IV, and MAC.
 			 */
-			drrw->drr_flags |= DRR_RAW_ENCRYPTED;
 			if (BP_SHOULD_BYTESWAP(bp))
 				drrw->drr_flags |= DRR_RAW_BYTESWAP;
 			zio_crypt_decode_params_bp(bp, drrw->drr_salt,
@@ -435,9 +434,9 @@ dump_spill(dmu_sendarg_t *dsp, const blkptr_t *bp, uint64_t object, void *data)
 	drrs->drr_toguid = dsp->dsa_toguid;
 
 	/* handle raw send fields */
-	if ((dsp->dsa_featureflags & DMU_BACKUP_FEATURE_RAW) != 0 &&
-	    BP_IS_PROTECTED(bp)) {
-		drrs->drr_flags |= DRR_RAW_ENCRYPTED;
+	if (dsp->dsa_featureflags & DMU_BACKUP_FEATURE_RAW) {
+		ASSERT(BP_IS_PROTECTED(bp));
+
 		if (BP_SHOULD_BYTESWAP(bp))
 			drrs->drr_flags |= DRR_RAW_BYTESWAP;
 		drrs->drr_compressiontype = BP_GET_COMPRESS(bp);
@@ -543,9 +542,9 @@ dump_dnode(dmu_sendarg_t *dsp, const blkptr_t *bp, uint64_t object,
 	    drro->drr_blksz > SPA_OLD_MAXBLOCKSIZE)
 		drro->drr_blksz = SPA_OLD_MAXBLOCKSIZE;
 
-	if ((dsp->dsa_featureflags & DMU_BACKUP_FEATURE_RAW) &&
-	    BP_IS_PROTECTED(bp)) {
-		drro->drr_flags |= DRR_RAW_ENCRYPTED;
+	if ((dsp->dsa_featureflags & DMU_BACKUP_FEATURE_RAW)) {
+		ASSERT(BP_IS_ENCRYPTED(bp));
+
 		if (BP_SHOULD_BYTESWAP(bp))
 			drro->drr_flags |= DRR_RAW_BYTESWAP;
 
@@ -602,7 +601,6 @@ dump_object_range(dmu_sendarg_t *dsp, const blkptr_t *bp, uint64_t firstobj,
 	drror->drr_firstobj = firstobj;
 	drror->drr_numslots = numslots;
 	drror->drr_toguid = dsp->dsa_toguid;
-	drror->drr_flags |= DRR_RAW_ENCRYPTED;
 	if (BP_SHOULD_BYTESWAP(bp))
 		drror->drr_flags |= DRR_RAW_BYTESWAP;
 	zio_crypt_decode_params_bp(bp, drror->drr_salt, drror->drr_iv);
@@ -1725,15 +1723,13 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
 				return (error);
 			}
 			if (!origin->ds_is_snapshot) {
-				dsl_dataset_rele_flags(origin,
-				    DS_HOLD_FLAG_DECRYPT, FTAG);
+				dsl_dataset_rele_flags(origin, dsflags, FTAG);
 				dsl_dataset_rele_flags(ds, dsflags, FTAG);
 				return (SET_ERROR(EINVAL));
 			}
 			if (dsl_dataset_phys(origin)->ds_guid != fromguid &&
 			    fromguid != 0) {
-				dsl_dataset_rele_flags(origin,
-				    DS_HOLD_FLAG_DECRYPT, FTAG);
+				dsl_dataset_rele_flags(origin, dsflags, FTAG);
 				dsl_dataset_rele_flags(ds, dsflags, FTAG);
 				return (SET_ERROR(ENODEV));
 			}
@@ -2136,6 +2132,7 @@ struct receive_writer_arg {
 	/* A map from guid to dataset to help handle dedup'd streams. */
 	avl_tree_t *guid_to_ds_map;
 	boolean_t resumable;
+	boolean_t raw;
 	uint64_t last_object, last_offset;
 	uint64_t bytes_read; /* bytes read when current record created */
 };
@@ -2170,6 +2167,7 @@ struct receive_arg  {
 	zio_cksum_t prev_cksum;
 	int err;
 	boolean_t byteswap;
+	boolean_t raw;
 	uint64_t featureflags;
 	/* Sorted list of objects not to issue prefetches for. */
 	struct objlist ignore_objlist;
@@ -2416,7 +2414,7 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
 		return (SET_ERROR(EINVAL));
 	}
 
-	if (DRR_IS_RAW_ENCRYPTED(drro->drr_flags)) {
+	if (rwa->raw) {
 		if (drro->drr_raw_bonuslen < drro->drr_bonuslen ||
 		    drro->drr_indblkshift > SPA_MAXBLOCKSHIFT ||
 		    drro->drr_nlevels > DN_MAX_LEVELS ||
@@ -2451,13 +2449,12 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
 		    drro->drr_bonuslen);
 
 		/* nblkptr will be bounded by the bonus size and type */
-		if (DRR_IS_RAW_ENCRYPTED(drro->drr_flags) &&
-		    nblkptr != drro->drr_nblkptr)
+		if (rwa->raw && nblkptr != drro->drr_nblkptr)
 			return (SET_ERROR(EINVAL));
 
 		if (drro->drr_blksz != doi.doi_data_block_size ||
 		    nblkptr < doi.doi_nblkptr ||
-		    (DRR_IS_RAW_ENCRYPTED(drro->drr_flags) &&
+		    (rwa->raw &&
 		    (indblksz != doi.doi_metadata_block_size ||
 		    drro->drr_nlevels < doi.doi_indirection))) {
 			err = dmu_free_long_range(rwa->os, drro->drr_object,
@@ -2492,17 +2489,20 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
 		    drro->drr_bonustype, drro->drr_bonuslen, tx);
 	}
 	if (err != 0) {
-		dmu_tx_abort(tx);
+		dmu_tx_commit(tx);
 		return (SET_ERROR(EINVAL));
 	}
 
+	if (rwa->raw)
+		VERIFY0(dmu_object_dirty_raw(rwa->os, drro->drr_object, tx));
+
 	dmu_object_set_checksum(rwa->os, drro->drr_object,
 	    drro->drr_checksumtype, tx);
 	dmu_object_set_compress(rwa->os, drro->drr_object,
 	    drro->drr_compress, tx);
 
 	/* handle more restrictive dnode structuring for raw recvs */
-	if (DRR_IS_RAW_ENCRYPTED(drro->drr_flags)) {
+	if (rwa->raw) {
 		/*
 		 * Set the indirect block shift and nlevels. This will not fail
 		 * because we ensured all of the blocks were free earlier if
@@ -2518,7 +2518,7 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
 		dmu_buf_t *db;
 		uint32_t flags = DMU_READ_NO_PREFETCH;
 
-		if (DRR_IS_RAW_ENCRYPTED(drro->drr_flags))
+		if (rwa->raw)
 			flags |= DMU_READ_NO_DECRYPT;
 
 		VERIFY0(dmu_bonus_hold_impl(rwa->os, drro->drr_object,
@@ -2532,7 +2532,7 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
 		 * Raw bonus buffers have their byteorder determined by the
 		 * DRR_OBJECT_RANGE record.
 		 */
-		if (rwa->byteswap && !DRR_IS_RAW_ENCRYPTED(drro->drr_flags)) {
+		if (rwa->byteswap && !rwa->raw) {
 			dmu_object_byteswap_t byteswap =
 			    DMU_OT_BYTESWAP(drro->drr_bonustype);
 			dmu_ot_byteswap[byteswap].ob_func(db->db_data,
@@ -2615,6 +2615,10 @@ receive_write(struct receive_writer_arg *rwa, struct drr_write *drrw,
 		dmu_tx_abort(tx);
 		return (err);
 	}
+
+	if (rwa->raw)
+		VERIFY0(dmu_object_dirty_raw(rwa->os, drrw->drr_object, tx));
+
 	if (rwa->byteswap && !arc_is_encrypted(abuf) &&
 	    arc_get_compression(abuf) == ZIO_COMPRESS_OFF) {
 		dmu_object_byteswap_t byteswap =
@@ -2680,9 +2684,8 @@ receive_write_byref(struct receive_writer_arg *rwa,
 		ref_os = rwa->os;
 	}
 
-	if (DRR_IS_RAW_ENCRYPTED(drrwbr->drr_flags)) {
+	if (rwa->raw)
 		flags |= DMU_READ_NO_DECRYPT;
-	}
 
 	/* may return either a regular db or an encrypted one */
 	err = dmu_buf_hold(ref_os, drrwbr->drr_refobject,
@@ -2700,7 +2703,8 @@ receive_write_byref(struct receive_writer_arg *rwa,
 		return (err);
 	}
 
-	if (DRR_IS_RAW_ENCRYPTED(drrwbr->drr_flags)) {
+	if (rwa->raw) {
+		VERIFY0(dmu_object_dirty_raw(rwa->os, drrwbr->drr_object, tx));
 		dmu_copy_from_buf(rwa->os, drrwbr->drr_object,
 		    drrwbr->drr_offset, dbp, tx);
 	} else {
@@ -2766,7 +2770,7 @@ receive_spill(struct receive_writer_arg *rwa, struct drr_spill *drrs,
 	    drrs->drr_length > spa_maxblocksize(dmu_objset_spa(rwa->os)))
 		return (SET_ERROR(EINVAL));
 
-	if (DRR_IS_RAW_ENCRYPTED(drrs->drr_flags)) {
+	if (rwa->raw) {
 		if (!DMU_OT_IS_VALID(drrs->drr_type) ||
 		    drrs->drr_compressiontype >= ZIO_COMPRESS_FUNCTIONS ||
 		    drrs->drr_compressed_size == 0)
@@ -2794,6 +2798,8 @@ receive_spill(struct receive_writer_arg *rwa, struct drr_spill *drrs,
 		return (err);
 	}
 	dmu_buf_will_dirty(db_spill, tx);
+	if (rwa->raw)
+		VERIFY0(dmu_object_dirty_raw(rwa->os, drrs->drr_object, tx));
 
 	if (db_spill->db_size < drrs->drr_length)
 		VERIFY(0 == dbuf_spill_set_blksz(db_spill,
@@ -2859,7 +2865,7 @@ receive_object_range(struct receive_writer_arg *rwa,
 	 */
 	if (drror->drr_numslots != DNODES_PER_BLOCK ||
 	    P2PHASE(drror->drr_firstobj, DNODES_PER_BLOCK) != 0 ||
-	    !DRR_IS_RAW_ENCRYPTED(drror->drr_flags))
+	    !rwa->raw)
 		return (SET_ERROR(EINVAL));
 
 	offset = drror->drr_firstobj * sizeof (dnode_phys_t);
@@ -3143,7 +3149,7 @@ receive_read_record(struct receive_arg *ra)
 		arc_buf_t *abuf;
 		boolean_t is_meta = DMU_OT_IS_METADATA(drrw->drr_type);
 
-		if (DRR_IS_RAW_ENCRYPTED(drrw->drr_flags)) {
+		if (ra->raw) {
 			boolean_t byteorder = ZFS_HOST_BYTEORDER ^
 			    !!DRR_IS_RAW_BYTESWAPPED(drrw->drr_flags) ^
 			    ra->byteswap;
@@ -3227,7 +3233,7 @@ receive_read_record(struct receive_arg *ra)
 		int len = DRR_SPILL_PAYLOAD_SIZE(drrs);
 
 		/* DRR_SPILL records are either raw or uncompressed */
-		if (DRR_IS_RAW_ENCRYPTED(drrs->drr_flags)) {
+		if (ra->raw) {
 			boolean_t byteorder = ZFS_HOST_BYTEORDER ^
 			    !!DRR_IS_RAW_BYTESWAPPED(drrs->drr_flags) ^
 			    ra->byteswap;
@@ -3529,6 +3535,7 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
 	rwa = kmem_zalloc(sizeof (*rwa), KM_SLEEP);
 
 	ra->byteswap = drc->drc_byteswap;
+	ra->raw = drc->drc_raw;
 	ra->cksum = drc->drc_cksum;
 	ra->vp = vp;
 	ra->voff = *voffp;
@@ -3556,16 +3563,23 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
 	featureflags = DMU_GET_FEATUREFLAGS(drc->drc_drrb->drr_versioninfo);
 	ra->featureflags = featureflags;
 
+	/* embedded data is incompatible with encrypted datasets */
+	if (ra->os->os_encrypted &&
+	    (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)) {
+		err = SET_ERROR(EINVAL);
+		goto out;
+	}
+
 	/* if this stream is dedup'ed, set up the avl tree for guid mapping */
 	if (featureflags & DMU_BACKUP_FEATURE_DEDUP) {
 		minor_t minor;
 
 		if (cleanup_fd == -1) {
-			ra->err = SET_ERROR(EBADF);
+			err = SET_ERROR(EBADF);
 			goto out;
 		}
-		ra->err = zfs_onexit_fd_hold(cleanup_fd, &minor);
-		if (ra->err != 0) {
+		err = zfs_onexit_fd_hold(cleanup_fd, &minor);
+		if (err != 0) {
 			cleanup_fd = -1;
 			goto out;
 		}
@@ -3579,12 +3593,12 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
 			err = zfs_onexit_add_cb(minor,
 			    free_guid_map_onexit, rwa->guid_to_ds_map,
 			    action_handlep);
-			if (ra->err != 0)
+			if (err != 0)
 				goto out;
 		} else {
 			err = zfs_onexit_cb_data(minor, *action_handlep,
 			    (void **)&rwa->guid_to_ds_map);
-			if (ra->err != 0)
+			if (err != 0)
 				goto out;
 		}
 
@@ -3640,6 +3654,7 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
 	rwa->os = ra->os;
 	rwa->byteswap = drc->drc_byteswap;
 	rwa->resumable = drc->drc_resumable;
+	rwa->raw = drc->drc_raw;
 
 	(void) thread_create(NULL, 0, receive_writer_thread, rwa, 0, curproc,
 	    TS_RUN, minclsyspri);