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 <behlendorf1@llnl.gov>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes #6512
Closes #6524
Closes #6545

diff --git a/include/sys/dmu.h b/include/sys/dmu.h
index 7c7e6dcbf808e0e8244c4175cfcacec4bb52c59a..60778289e9b49ca92578425abe7e4b14d052ba5e 100644 (file)
--- 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 90458827126b4b82427c725ea18f5dc4ee822549..6924280c4a4785232af55526921f8ec234dc8a9f 100644 (file)
--- 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 c6ad06951be4cda4eebc6a7b6ab1cdbe097e56d3..3146b4e617c6fd1a310efdb3375e30ffe7916707 100644 (file)
--- 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 f344eb943a130fb90df7d885d0bb050813346a50..46306e9cc8ff639de75dc1509cd8bacaf95486ae 100644 (file)
--- 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 5b90818f4ee7930d2960ce74f106ed35cea7e59d..1eb35b935174ab9a5c53357e29daacfe59e3b7af 100644 (file)
--- 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 aca50197b131bc0d540680d1732929020f7c97ae..359ef99d35fdb243269dd0f79978955e6489c319 100644 (file)
--- 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);