svn commit: r359303 - in head: cddl/contrib/opensolaris/lib/libzfs/common sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/cddl/contrib/opensolaris/uts/common/sys/fs

Ryan Moeller freqlabs at FreeBSD.org
Wed Mar 25 16:05:33 UTC 2020


Author: freqlabs
Date: Wed Mar 25 15:56:18 2020
New Revision: 359303
URL: https://svnweb.freebsd.org/changeset/base/359303

Log:
  MFOpenZFS: ZVOLs should not be allowed to have children
  
  zfs create, receive and rename can bypass this hierarchy rule. Update
  both userland and kernel module to prevent this issue and use pyzfs
  unit tests to exercise the ioctls directly.
  
  Note: this commit slightly changes zfs_ioc_create() ABI. This allow to
  differentiate a generic error (EINVAL) from the specific case where we
  tried to create a dataset below a ZVOL (ZFS_ERR_WRONG_PARENT).
  
  Reviewed-by: Paul Dagnelie <pcd at delphix.com>
  Reviewed-by: Matt Ahrens <mahrens at delphix.com>
  Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
  Reviewed-by: Tom Caputi <tcaputi at datto.com>
  Signed-off-by: loli10K <ezomori.nozomu at gmail.com>
  
  Approved by:	mav (mentor)
  MFC after:	2 weeks
  Sponsored by:	iXsystems, Inc.
  openzfs/zfs at d8d418ff0cc90776182534bce10b01e9487b63e4

Modified:
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
  head/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h	Wed Mar 25 15:56:18 2020	(r359303)
@@ -141,6 +141,7 @@ typedef enum zfs_error {
 	EZFS_TOOMANY,		/* argument list too long */
 	EZFS_INITIALIZING,	/* currently initializing */
 	EZFS_NO_INITIALIZE,	/* no active initialize */
+	EZFS_WRONG_PARENT,	/* invalid parent dataset (e.g ZVOL) */
 	EZFS_UNKNOWN
 } zfs_error_t;
 

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c	Wed Mar 25 15:56:18 2020	(r359303)
@@ -3654,11 +3654,6 @@ zfs_create(libzfs_handle_t *hdl, const char *path, zfs
 			    "no such parent '%s'"), parent);
 			return (zfs_error(hdl, EZFS_NOENT, errbuf));
 
-		case EINVAL:
-			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-			    "parent '%s' is not a filesystem"), parent);
-			return (zfs_error(hdl, EZFS_BADTYPE, errbuf));
-
 		case ENOTSUP:
 			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
 			    "pool must be upgraded to set this "

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c	Wed Mar 25 15:56:18 2020	(r359303)
@@ -28,6 +28,7 @@
  * Copyright 2015, OmniTI Computer Consulting, Inc. All rights reserved.
  * Copyright (c) 2014 Integros [integros.com]
  * Copyright 2016 Igor Kozhukhov <ikozhukhov at gmail.com>
+ * Copyright (c) 2018, loli10K <ezomori.nozomu at gmail.com>. All rights reserved.
  * Copyright (c) 2019 Datto Inc.
  */
 
@@ -3375,6 +3376,7 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const 
 		 *  - we are resuming a failed receive.
 		 */
 		if (stream_wantsnewfs) {
+			boolean_t is_volume = drrb->drr_type == DMU_OST_ZVOL;
 			if (!flags->force) {
 				zcmd_free_nvlists(&zc);
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
@@ -3392,6 +3394,25 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const 
 				    zc.zc_name);
 				return (zfs_error(hdl, EZFS_EXISTS, errbuf));
 			}
+			if (is_volume && strrchr(zc.zc_name, '/') == NULL) {
+				zcmd_free_nvlists(&zc);
+				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+				    "destination '%s' is the root dataset\n"
+				    "cannot overwrite with a ZVOL"),
+				    zc.zc_name);
+				return (zfs_error(hdl, EZFS_EXISTS, errbuf));
+			}
+			if (is_volume &&
+			    ioctl(hdl->libzfs_fd, ZFS_IOC_DATASET_LIST_NEXT,
+			    &zc) == 0) {
+				zcmd_free_nvlists(&zc);
+				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+				    "destination has children (eg. %s)\n"
+				    "cannot overwrite with a ZVOL"),
+				    zc.zc_name);
+				return (zfs_error(hdl, EZFS_WRONG_PARENT,
+				    errbuf));
+			}
 		}
 
 		if ((zhp = zfs_open(hdl, zc.zc_name,
@@ -3441,6 +3462,8 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const 
 
 		zfs_close(zhp);
 	} else {
+		zfs_handle_t *zhp;
+
 		/*
 		 * Destination filesystem does not exist.  Therefore we better
 		 * be creating a new filesystem (either from a full backup, or
@@ -3467,6 +3490,21 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const 
 			zcmd_free_nvlists(&zc);
 			return (zfs_error(hdl, EZFS_BADRESTORE, errbuf));
 		}
+
+		/* validate parent */
+		zhp = zfs_open(hdl, zc.zc_name, ZFS_TYPE_DATASET);
+		if (zhp == NULL) {
+			zcmd_free_nvlists(&zc);
+			return (zfs_error(hdl, EZFS_BADRESTORE, errbuf));
+		}
+		if (zfs_get_type(zhp) != ZFS_TYPE_FILESYSTEM) {
+			zcmd_free_nvlists(&zc);
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "parent '%s' is not a filesystem"), zc.zc_name);
+			zfs_close(zhp);
+			return (zfs_error(hdl, EZFS_WRONG_PARENT, errbuf));
+		}
+		zfs_close(zhp);
 
 		newfs = B_TRUE;
 	}

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c	Wed Mar 25 15:56:18 2020	(r359303)
@@ -265,6 +265,8 @@ libzfs_error_description(libzfs_handle_t *hdl)
 	case EZFS_NO_INITIALIZE:
 		return (dgettext(TEXT_DOMAIN, "there is no active "
 		    "initialization"));
+	case EZFS_WRONG_PARENT:
+		return (dgettext(TEXT_DOMAIN, "invalid parent dataset"));
 	case EZFS_UNKNOWN:
 		return (dgettext(TEXT_DOMAIN, "unknown error"));
 	default:
@@ -536,6 +538,9 @@ zpool_standard_error_fmt(libzfs_handle_t *hdl, int err
 		break;
 	case ZFS_ERR_VDEV_TOO_BIG:
 		zfs_verror(hdl, EZFS_VDEV_TOO_BIG, fmt, ap);
+		break;
+	case ZFS_ERR_WRONG_PARENT:
+		zfs_verror(hdl, EZFS_WRONG_PARENT, fmt, ap);
 		break;
 	default:
 		zfs_error_aux(hdl, strerror(error));

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c	Wed Mar 25 15:56:18 2020	(r359303)
@@ -28,6 +28,7 @@
  * Copyright (c) 2015, STRATO AG, Inc. All rights reserved.
  * Copyright (c) 2014 Integros [integros.com]
  * Copyright 2017 Nexenta Systems, Inc.
+ * Copyright (c) 2018, loli10K <ezomori.nozomu at gmail.com>. All rights reserved.
  */
 
 /* Portions Copyright 2010 Robert Milkowski */
@@ -971,6 +972,8 @@ dmu_objset_create_check(void *arg, dmu_tx_t *tx)
 	dmu_objset_create_arg_t *doca = arg;
 	dsl_pool_t *dp = dmu_tx_pool(tx);
 	dsl_dir_t *pdd;
+	dsl_dataset_t *parentds;
+	objset_t *parentos;
 	const char *tail;
 	int error;
 
@@ -992,6 +995,30 @@ dmu_objset_create_check(void *arg, dmu_tx_t *tx)
 	}
 	error = dsl_fs_ss_limit_check(pdd, 1, ZFS_PROP_FILESYSTEM_LIMIT, NULL,
 	    doca->doca_cred);
+	if (error != 0) {
+		dsl_dir_rele(pdd, FTAG);
+		return (error);
+	}
+
+	/* can't create below anything but filesystems (eg. no ZVOLs) */
+	error = dsl_dataset_hold_obj(pdd->dd_pool,
+	    dsl_dir_phys(pdd)->dd_head_dataset_obj, FTAG, &parentds);
+	if (error != 0) {
+		dsl_dir_rele(pdd, FTAG);
+		return (error);
+	}
+	error = dmu_objset_from_ds(parentds, &parentos);
+	if (error != 0) {
+		dsl_dataset_rele(parentds, FTAG);
+		dsl_dir_rele(pdd, FTAG);
+		return (error);
+	}
+	if (dmu_objset_type(parentos) != DMU_OST_ZFS) {
+		dsl_dataset_rele(parentds, FTAG);
+		dsl_dir_rele(pdd, FTAG);
+		return (SET_ERROR(ZFS_ERR_WRONG_PARENT));
+	}
+	dsl_dataset_rele(parentds, FTAG);
 	dsl_dir_rele(pdd, FTAG);
 
 	return (error);

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c	Wed Mar 25 15:56:18 2020	(r359303)
@@ -27,6 +27,7 @@
  * Copyright 2014 HybridCluster. All rights reserved.
  * Copyright 2016 RackTop Systems.
  * Copyright (c) 2014 Integros [integros.com]
+ * Copyright (c) 2018, loli10K <ezomori.nozomu at gmail.com>. All rights reserved.
  */
 
 #include <sys/dmu.h>
@@ -1308,6 +1309,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *d
     uint64_t fromguid)
 {
 	uint64_t val;
+	uint64_t children;
 	int error;
 	dsl_pool_t *dp = ds->ds_dir->dd_pool;
 
@@ -1329,6 +1331,15 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *d
 	if (error != ENOENT)
 		return (error == 0 ? SET_ERROR(EEXIST) : error);
 
+	/* must not have children if receiving a ZVOL */
+	error = zap_count(dp->dp_meta_objset,
+	    dsl_dir_phys(ds->ds_dir)->dd_child_dir_zapobj, &children);
+	if (error != 0)
+		return (error);
+	if (drba->drba_cookie->drc_drrb->drr_type != DMU_OST_ZFS &&
+	    children > 0)
+		return (SET_ERROR(ZFS_ERR_WRONG_PARENT));
+
 	/*
 	 * Check snapshot limit before receiving. We'll recheck again at the
 	 * end, but might as well abort before receiving if we're already over
@@ -1466,6 +1477,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
 	} else if (error == ENOENT) {
 		/* target fs does not exist; must be a full backup or clone */
 		char buf[ZFS_MAX_DATASET_NAME_LEN];
+		objset_t *os;
 
 		/*
 		 * If it's a non-clone incremental, we are missing the
@@ -1510,6 +1522,17 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
 			return (error);
 		}
 
+		/* can't recv below anything but filesystems (eg. no ZVOLs) */
+		error = dmu_objset_from_ds(ds, &os);
+		if (error != 0) {
+			dsl_dataset_rele(ds, FTAG);
+			return (error);
+		}
+		if (dmu_objset_type(os) != DMU_OST_ZFS) {
+			dsl_dataset_rele(ds, FTAG);
+			return (SET_ERROR(ZFS_ERR_WRONG_PARENT));
+		}
+
 		if (drba->drba_origin != NULL) {
 			dsl_dataset_t *origin;
 			error = dsl_dataset_hold(dp, drba->drba_origin,
@@ -1531,6 +1554,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
 			}
 			dsl_dataset_rele(origin, FTAG);
 		}
+
 		dsl_dataset_rele(ds, FTAG);
 		error = 0;
 	}

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c	Wed Mar 25 15:56:18 2020	(r359303)
@@ -26,6 +26,7 @@
  * Copyright (c) 2014 Joyent, Inc. All rights reserved.
  * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved.
  * Copyright 2015 Nexenta Systems, Inc. All rights reserved.
+ * Copyright (c) 2018, loli10K <ezomori.nozomu at gmail.com>. All rights reserved.
  */
 
 #include <sys/dmu.h>
@@ -1852,6 +1853,8 @@ dsl_dir_rename_check(void *arg, dmu_tx_t *tx)
 	dsl_pool_t *dp = dmu_tx_pool(tx);
 	dsl_dir_t *dd, *newparent;
 	dsl_valid_rename_arg_t dvra;
+	dsl_dataset_t *parentds;
+	objset_t *parentos;
 	const char *mynewname;
 	int error;
 
@@ -1881,6 +1884,29 @@ dsl_dir_rename_check(void *arg, dmu_tx_t *tx)
 		dsl_dir_rele(dd, FTAG);
 		return (SET_ERROR(EEXIST));
 	}
+
+	/* can't rename below anything but filesystems (eg. no ZVOLs) */
+	error = dsl_dataset_hold_obj(newparent->dd_pool,
+	    dsl_dir_phys(newparent)->dd_head_dataset_obj, FTAG, &parentds);
+	if (error != 0) {
+		dsl_dir_rele(newparent, FTAG);
+		dsl_dir_rele(dd, FTAG);
+		return (error);
+	}
+	error = dmu_objset_from_ds(parentds, &parentos);
+	if (error != 0) {
+		dsl_dataset_rele(parentds, FTAG);
+		dsl_dir_rele(newparent, FTAG);
+		dsl_dir_rele(dd, FTAG);
+		return (error);
+	}
+	if (dmu_objset_type(parentos) != DMU_OST_ZFS) {
+		dsl_dataset_rele(parentds, FTAG);
+		dsl_dir_rele(newparent, FTAG);
+		dsl_dir_rele(dd, FTAG);
+		return (error);
+	}
+	dsl_dataset_rele(parentds, FTAG);
 
 	ASSERT3U(strnlen(ddra->ddra_newname, ZFS_MAX_DATASET_NAME_LEN),
 	    <, ZFS_MAX_DATASET_NAME_LEN);

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c	Wed Mar 25 15:56:18 2020	(r359303)
@@ -33,6 +33,7 @@
  * Copyright (c) 2014 Integros [integros.com]
  * Copyright 2016 Toomas Soome <tsoome at me.com>
  * Copyright 2017 RackTop Systems.
+ * Copyright (c) 2018, loli10K <ezomori.nozomu at gmail.com>. All rights reserved.
  * Copyright (c) 2019 Datto Inc.
  */
 
@@ -3172,8 +3173,9 @@ zfs_fill_zplprops_impl(objset_t *os, uint64_t zplver,
 
 	ASSERT(zplprops != NULL);
 
+	/* parent dataset must be a filesystem */
 	if (os != NULL && os->os_phys->os_type != DMU_OST_ZFS)
-		return (SET_ERROR(EINVAL));
+		return (SET_ERROR(ZFS_ERR_WRONG_PARENT));
 
 	/*
 	 * Pull out creator prop choices, if any.
@@ -3249,15 +3251,11 @@ zfs_fill_zplprops(const char *dataset, nvlist_t *creat
 	uint64_t zplver = ZPL_VERSION;
 	objset_t *os = NULL;
 	char parentname[ZFS_MAX_DATASET_NAME_LEN];
-	char *cp;
 	spa_t *spa;
 	uint64_t spa_vers;
 	int error;
 
-	(void) strlcpy(parentname, dataset, sizeof (parentname));
-	cp = strrchr(parentname, '/');
-	ASSERT(cp != NULL);
-	cp[0] = '\0';
+	zfs_get_parent(dataset, parentname, sizeof (parentname));
 
 	if ((error = spa_open(dataset, &spa, FTAG)) != 0)
 		return (error);

Modified: head/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h	Wed Mar 25 15:29:01 2020	(r359302)
+++ head/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h	Wed Mar 25 15:56:18 2020	(r359303)
@@ -1063,7 +1063,8 @@ typedef enum {
 	ZFS_ERR_DISCARDING_CHECKPOINT,
 	ZFS_ERR_NO_CHECKPOINT,
 	ZFS_ERR_DEVRM_IN_PROGRESS,
-	ZFS_ERR_VDEV_TOO_BIG
+	ZFS_ERR_VDEV_TOO_BIG,
+	ZFS_ERR_WRONG_PARENT,
 } zfs_errno_t;
 
 /*


More information about the svn-src-head mailing list