From 9b8407638da71ea9f4afb21375f991869f19811f Mon Sep 17 00:00:00 2001 From: Tom Caputi Date: Wed, 23 Aug 2017 19:54:24 -0400 Subject: [PATCH] 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 Signed-off-by: Tom Caputi Closes #6512 Closes #6524 Closes #6545 --- include/sys/dmu.h | 5 ++- include/sys/zfs_ioctl.h | 16 +++---- lib/libzfs/libzfs_sendrecv.c | 6 +++ man/man8/zfs.8 | 3 +- module/zfs/dmu.c | 19 ++++++++ module/zfs/dmu_send.c | 87 +++++++++++++++++++++--------------- 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); -- 2.40.0