several fixes and improvements after second round of review

include/libzfs.h

@@ -501,9 +501,9 @@ extern nvlist_t *zfs_get_clones_nvl(zfs_handle_t *);
  */
 extern int zfs_crypto_is_encryption_root(zfs_handle_t *, boolean_t *);
 extern int zfs_crypto_create(libzfs_handle_t *, char *, nvlist_t *, nvlist_t *,
-    nvlist_t **);
+    uint8_t **, uint_t *);
 extern int zfs_crypto_clone(libzfs_handle_t *, zfs_handle_t *, char *,
-    nvlist_t *, nvlist_t **);
+    nvlist_t *, uint8_t **, uint_t *);
 extern int zfs_crypto_attempt_load_keys(libzfs_handle_t *, char *);
 extern int zfs_crypto_load_key(zfs_handle_t *, boolean_t, char *);
 extern int zfs_crypto_unload_key(zfs_handle_t *);

include/libzfs_core.h

@@ -47,15 +47,16 @@ enum lzc_dataset_type {
 };
 
 int lzc_snapshot(nvlist_t *, nvlist_t *, nvlist_t **);
-int lzc_create(const char *, enum lzc_dataset_type, nvlist_t *, nvlist_t *);
-int lzc_clone(const char *, const char *, nvlist_t *, nvlist_t *);
+int lzc_create(const char *, enum lzc_dataset_type, nvlist_t *, uint8_t *,
+    uint_t);
+int lzc_clone(const char *, const char *, nvlist_t *, uint8_t *, uint_t);
 int lzc_destroy_snaps(nvlist_t *, boolean_t, nvlist_t **);
 int lzc_bookmark(nvlist_t *, nvlist_t **);
 int lzc_get_bookmarks(const char *, nvlist_t *, nvlist_t **);
 int lzc_destroy_bookmarks(nvlist_t *, nvlist_t **);
-int lzc_load_key(const char *, boolean_t, nvlist_t *);
+int lzc_load_key(const char *, boolean_t, uint8_t *, uint_t);
 int lzc_unload_key(const char *);
-int lzc_change_key(const char *, nvlist_t *, nvlist_t *);
+int lzc_change_key(const char *, nvlist_t *, uint8_t *, uint_t);
 
 int lzc_snaprange_space(const char *, const char *, uint64_t *);
 

include/sys/dmu.h

@@ -289,6 +289,9 @@ void zfs_znode_byteswap(void *buf, size_t size);
 /*
  * Public routines to create, destroy, open, and close objsets.
  */
+typedef void dmu_objset_create_sync_func_t(objset_t *os, void *arg,
+    cred_t *cr, dmu_tx_t *tx);
+
 int dmu_objset_hold(const char *name, void *tag, objset_t **osp);
 int dmu_objset_own(const char *name, dmu_objset_type_t type,
     boolean_t readonly, boolean_t key_required, void *tag, objset_t **osp);
@@ -298,8 +301,8 @@ int dmu_objset_open_ds(struct dsl_dataset *ds, objset_t **osp);
 
 void dmu_objset_evict_dbufs(objset_t *os);
 int dmu_objset_create(const char *name, dmu_objset_type_t type, uint64_t flags,
-    struct dsl_crypto_params *dcp, void (*func)(objset_t *os, void *arg,
-	cred_t *cr, dmu_tx_t *tx), void *arg);
+    struct dsl_crypto_params *dcp, dmu_objset_create_sync_func_t func,
+    void *arg);
 int dmu_objset_clone(const char *name, const char *origin,
 	struct dsl_crypto_params *dcp);
 int dsl_destroy_snapshots_nvl(struct nvlist *snaps, boolean_t defer,

include/sys/dsl_crypt.h

@@ -157,6 +157,7 @@ int spa_keystore_lookup_key(spa_t *spa, uint64_t dsobj, void *tag,
     dsl_crypto_key_t **dck_out);
 
 int spa_keystore_rewrap(const char *dsname, dsl_crypto_params_t *dcp);
+int dsl_dir_rename_crypt_check(dsl_dir_t *dd, dsl_dir_t *newparent);
 int dmu_objset_create_crypt_check(dsl_dir_t *parentdd, dsl_dir_t *origindd,
     dsl_crypto_params_t *dcp);
 void dsl_dataset_create_crypt_sync(uint64_t dsobj, dsl_dir_t *dd,

include/sys/dsl_dataset.h

@@ -248,25 +248,28 @@ dsl_dataset_phys(dsl_dataset_t *ds)
 	((dsl_dataset_phys(ds)->ds_flags & DS_FLAG_UNIQUE_ACCURATE) != 0)
 
 /* flags for holding the dataset */
-#define	DS_HOLD_FLAG_DECRYPT	(1 << 0) /* needs access encrypted data */
+typedef enum ds_hold_flags {
+	DS_HOLD_FLAG_DECRYPT	= 1 << 0 /* needs access encrypted data */
+} ds_hold_flags_t;
 
 int dsl_dataset_hold(struct dsl_pool *dp, const char *name, void *tag,
     dsl_dataset_t **dsp);
-int dsl_dataset_hold_flags(struct dsl_pool *dp, const char *name, int flags,
-    void *tag, dsl_dataset_t **dsp);
+int dsl_dataset_hold_flags(struct dsl_pool *dp, const char *name,
+    ds_hold_flags_t flags, void *tag, dsl_dataset_t **dsp);
 boolean_t dsl_dataset_try_add_ref(struct dsl_pool *dp, dsl_dataset_t *ds,
     void *tag);
 int dsl_dataset_hold_obj(struct dsl_pool *dp, uint64_t dsobj, void *tag,
     dsl_dataset_t **);
-int dsl_dataset_hold_obj_flags(struct dsl_pool *dp, uint64_t dsobj, int flags,
-    void *tag, dsl_dataset_t **);
+int dsl_dataset_hold_obj_flags(struct dsl_pool *dp, uint64_t dsobj,
+    ds_hold_flags_t flags, void *tag, dsl_dataset_t **);
 void dsl_dataset_rele(dsl_dataset_t *ds, void *tag);
-void dsl_dataset_rele_flags(dsl_dataset_t *ds, int flags, void *tag);
-int dsl_dataset_own(struct dsl_pool *dp, const char *name, int flags,
-    void *tag, dsl_dataset_t **dsp);
-int dsl_dataset_own_obj(struct dsl_pool *dp, uint64_t dsobj, int flags,
-    void *tag, dsl_dataset_t **dsp);
-void dsl_dataset_disown(dsl_dataset_t *ds, int flags, void *tag);
+void dsl_dataset_rele_flags(dsl_dataset_t *ds, ds_hold_flags_t flags,
+    void *tag);
+int dsl_dataset_own(struct dsl_pool *dp, const char *name,
+    ds_hold_flags_t flags, void *tag, dsl_dataset_t **dsp);
+int dsl_dataset_own_obj(struct dsl_pool *dp, uint64_t dsobj,
+    ds_hold_flags_t flags, void *tag, dsl_dataset_t **dsp);
+void dsl_dataset_disown(dsl_dataset_t *ds, ds_hold_flags_t flags, void *tag);
 void dsl_dataset_name(dsl_dataset_t *ds, char *name);
 int dsl_dataset_namelen(dsl_dataset_t *ds);
 boolean_t dsl_dataset_has_owner(dsl_dataset_t *ds);

include/sys/spa.h

@@ -294,10 +294,7 @@ typedef struct zio_cksum_salt {
  * normal 3. The last 32 bits are stored in the upper bits of what is usually
  * the fill count. Note that only level 0 bocks are ever encrypted (or -2 in
  * the case of ZIL blocks), which allows us to guarantee that these 32 bits
- * are not trampled over by other code (see zio_crypt.c for details). The
- * underlying encryption code looks at these values as byte arrays rather than
- * uint64_t's, so these are always stored in little-endian format regardless
- * of the host machine's native endianness.
+ * are not trampled over by other code (see zio_crypt.c for details).
  */
 
 /*

include/sys/zio_crypt.h

@@ -152,7 +152,7 @@ void zio_crypt_encode_params_bp(blkptr_t *bp, uint8_t *salt, uint8_t *iv);
 void zio_crypt_decode_params_bp(const blkptr_t *bp, uint8_t *salt, uint8_t *iv);
 void zio_crypt_encode_mac_bp(blkptr_t *bp, uint8_t *mac);
 void zio_crypt_decode_mac_bp(const blkptr_t *bp, uint8_t *mac);
-void zio_crypt_encode_mac_zil(const void *data, uint8_t *mac);
+void zio_crypt_encode_mac_zil(void *data, uint8_t *mac);
 void zio_crypt_decode_mac_zil(const void *data, uint8_t *mac);
 void zio_crypt_copy_dnode_bonus(abd_t *src_abd, uint8_t *dst, uint_t datalen);
 

lib/libzfs/libzfs_crypto.c

@@ -30,7 +30,7 @@
 #include "zfeature_common.h"
 
 /*
- * User keys are used to decrypt the master encyrption keys of a dataset. This
+ * User keys are used to decrypt the master encryption keys of a dataset. This
  * indirection allows a user to change his / her access key without having to
  * re-encrypt the entire dataset. User keys can be provided in one of several
  * ways. Raw keys are simlply given to the kernel as is. Similarly, hex keys
@@ -624,7 +624,7 @@ encryption_feature_is_enabled(zpool_handle_t *zph)
 static int
 populate_create_encryption_params_nvlists(libzfs_handle_t *hdl,
     zfs_handle_t *zhp, boolean_t newkey, zfs_keyformat_t keyformat,
-    char *keylocation, nvlist_t *props, nvlist_t *hidden_args)
+    char *keylocation, nvlist_t *props, uint8_t **wkeydata, uint_t *wkeylen)
 {
 	int ret;
 	uint64_t iters, salt = 0;
@@ -686,22 +686,20 @@ populate_create_encryption_params_nvlists(libzfs_handle_t *hdl,
 	if (ret != 0)
 		goto error;
 
-	/* add the derived key to the properties list */
-	ret = nvlist_add_uint8_array(hidden_args, "wkeydata", key_data,
-	    WRAPPING_KEY_LEN);
-	if (ret != 0)
-		goto error;
-
 	free(key_material);
-	free(key_data);
 
+	*wkeydata = key_data;
+	*wkeylen = WRAPPING_KEY_LEN;
 	return (0);
 
 error:
 	if (key_material != NULL)
 		free(key_material);
 	if (key_data != NULL)
 		free(key_data);
+
+	*wkeydata = NULL;
+	*wkeylen = 0;
 	return (ret);
 }
 
@@ -770,15 +768,16 @@ zfs_crypto_is_encryption_root(zfs_handle_t *zhp, boolean_t *enc_root)
 
 int
 zfs_crypto_create(libzfs_handle_t *hdl, char *parent_name, nvlist_t *props,
-    nvlist_t *pool_props, nvlist_t **hidden_args)
+    nvlist_t *pool_props, uint8_t **wkeydata_out, uint_t *wkeylen_out)
 {
 	int ret;
 	char errbuf[1024];
 	uint64_t crypt = ZIO_CRYPT_INHERIT, pcrypt = ZIO_CRYPT_INHERIT;
 	uint64_t keyformat = ZFS_KEYFORMAT_NONE;
 	char *keylocation = NULL;
 	zfs_handle_t *pzhp = NULL;
-	nvlist_t *ha = NULL;
+	uint8_t *wkeydata = NULL;
+	uint_t wkeylen = 0;
 	boolean_t local_crypt = B_TRUE;
 
 	(void) snprintf(errbuf, sizeof (errbuf),
@@ -800,9 +799,10 @@ zfs_crypto_create(libzfs_handle_t *hdl, char *parent_name, nvlist_t *props,
 		/* get a reference to parent dataset */
 		pzhp = make_dataset_handle(hdl, parent_name);
 		if (pzhp == NULL) {
+			ret = ENOENT;
 			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
 			    "Failed to lookup parent."));
-			return (ENOENT);
+			goto out;
 		}
 
 		/* Lookup parent's crypt */
@@ -909,39 +909,42 @@ zfs_crypto_create(libzfs_handle_t *hdl, char *parent_name, nvlist_t *props,
 	 * encryption root. Populate the encryption params.
 	 */
 	if (keylocation != NULL) {
-		ha = fnvlist_alloc();
-
 		ret = populate_create_encryption_params_nvlists(hdl, NULL,
-		    B_FALSE, keyformat, keylocation, props, ha);
+		    B_FALSE, keyformat, keylocation, props, &wkeydata,
+		    &wkeylen);
 		if (ret != 0)
 			goto out;
 	}
 
 	if (pzhp != NULL)
 		zfs_close(pzhp);
 
-	*hidden_args = ha;
+	*wkeydata_out = wkeydata;
+	*wkeylen_out = wkeylen;
 	return (0);
 
 out:
 	if (pzhp != NULL)
 		zfs_close(pzhp);
-	if (ha != NULL)
-		nvlist_free(ha);
+	if (wkeydata != NULL)
+		free(wkeydata);
 
-	*hidden_args = NULL;
+	*wkeydata_out = NULL;
+	*wkeylen_out = 0;
 	return (ret);
 }
 
 int
 zfs_crypto_clone(libzfs_handle_t *hdl, zfs_handle_t *origin_zhp,
-    char *parent_name, nvlist_t *props, nvlist_t **hidden_args)
+    char *parent_name, nvlist_t *props, uint8_t **wkeydata_out,
+    uint_t *wkeylen_out)
 {
 	int ret;
 	char errbuf[1024];
 	uint64_t keyformat = ZFS_KEYFORMAT_NONE;
 	char *keylocation = NULL;
-	nvlist_t *ha = NULL;
+	uint8_t *wkeydata = NULL;
+	uint_t wkeylen = 0;
 	zfs_handle_t *pzhp = NULL;
 	uint64_t crypt, pcrypt, ocrypt, okey_status;
 
@@ -1047,26 +1050,27 @@ zfs_crypto_clone(libzfs_handle_t *hdl, zfs_handle_t *origin_zhp,
 
 	/* prepare the key if needed */
 	if (keylocation != NULL) {
-		ha = fnvlist_alloc();
-
 		ret = populate_create_encryption_params_nvlists(hdl, NULL,
-		    B_FALSE, keyformat, keylocation, props, ha);
+		    B_FALSE, keyformat, keylocation, props, &wkeydata,
+		    &wkeylen);
 		if (ret != 0)
 			goto out;
 	}
 
 	zfs_close(pzhp);
 
-	*hidden_args = ha;
+	*wkeydata_out = wkeydata;
+	*wkeylen_out = wkeylen;
 	return (0);
 
 out:
 	if (pzhp != NULL)
 		zfs_close(pzhp);
-	if (ha != NULL)
-		nvlist_free(ha);
+	if (wkeydata != NULL)
+		free(wkeydata);
 
-	*hidden_args = NULL;
+	*wkeydata_out = NULL;
+	*wkeylen_out = 0;
 	return (ret);
 }
 
@@ -1158,7 +1162,6 @@ zfs_crypto_load_key(zfs_handle_t *zhp, boolean_t noop, char *alt_keylocation)
 	char *keylocation = NULL;
 	uint8_t *key_material = NULL, *key_data = NULL;
 	size_t key_material_len;
-	nvlist_t *crypto_args = NULL;
 	zprop_source_t keylocation_srctype;
 	boolean_t can_retry = B_FALSE, correctible = B_FALSE;
 
@@ -1246,15 +1249,8 @@ zfs_crypto_load_key(zfs_handle_t *zhp, boolean_t noop, char *alt_keylocation)
 
 	correctible = B_FALSE;
 
-	/* put the key in an nvlist and pass to the ioctl */
-	crypto_args = fnvlist_alloc();
-
-	ret = nvlist_add_uint8_array(crypto_args, "wkeydata", key_data,
-	    WRAPPING_KEY_LEN);
-	if (ret != 0)
-		goto error;
-
-	ret = lzc_load_key(zhp->zfs_name, noop, crypto_args);
+	/* pass the wrapping key and noop flag to the ioctl */
+	ret = lzc_load_key(zhp->zfs_name, noop, key_data, WRAPPING_KEY_LEN);
 	if (ret != 0) {
 		switch (ret) {
 		case EINVAL:
@@ -1280,7 +1276,6 @@ zfs_crypto_load_key(zfs_handle_t *zhp, boolean_t noop, char *alt_keylocation)
 		goto error;
 	}
 
-	nvlist_free(crypto_args);
 	free(key_material);
 	free(key_data);
 
@@ -1292,8 +1287,6 @@ zfs_crypto_load_key(zfs_handle_t *zhp, boolean_t noop, char *alt_keylocation)
 		free(key_material);
 	if (key_data != NULL)
 		free(key_data);
-	if (crypto_args != NULL)
-		nvlist_free(crypto_args);
 
 	/*
 	 * Here we decide if it is ok to allow the user to retry entering their
@@ -1449,7 +1442,8 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey)
 	char errbuf[1024];
 	boolean_t is_encroot;
 	nvlist_t *props = NULL;
-	nvlist_t *crypto_args = NULL;
+	uint8_t *wkeydata = NULL;
+	uint_t wkeylen = 0;
 	uint64_t crypt, pcrypt, keystatus, pkeystatus;
 	uint64_t keyformat = ZFS_KEYFORMAT_NONE;
 	zfs_handle_t *pzhp = NULL;
@@ -1514,11 +1508,10 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey)
 			keylocation = prop_keylocation;
 		}
 
-		/* populate an nvlist with the new wrapping key */
-		crypto_args = fnvlist_alloc();
-
+		/* fetch the new wrapping key and associated properties */
 		ret = populate_create_encryption_params_nvlists(zhp->zfs_hdl,
-		    zhp, B_TRUE, keyformat, keylocation, props, crypto_args);
+		    zhp, B_TRUE, keyformat, keylocation, props, &wkeydata,
+		    &wkeylen);
 		if (ret != 0)
 			goto error;
 	} else {
@@ -1579,7 +1572,7 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey)
 	}
 
 	/* call the ioctl */
-	ret = lzc_change_key(zhp->zfs_name, props, crypto_args);
+	ret = lzc_change_key(zhp->zfs_name, props, wkeydata, wkeylen);
 	if (ret != 0) {
 		switch (ret) {
 		case EINVAL:
@@ -1598,8 +1591,8 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey)
 		zfs_close(pzhp);
 	if (props != NULL)
 		nvlist_free(props);
-	if (crypto_args != NULL)
-		nvlist_free(crypto_args);
+	if (wkeydata != NULL)
+		free(wkeydata);
 
 	return (ret);
 
@@ -1608,8 +1601,8 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey)
 		zfs_close(pzhp);
 	if (props != NULL)
 		nvlist_free(props);
-	if (crypto_args != NULL)
-		nvlist_free(crypto_args);
+	if (wkeydata != NULL)
+		free(wkeydata);
 
 	zfs_error(zhp->zfs_hdl, EZFS_CRYPTOFAILED, errbuf);
 	return (ret);