OS-5409 sdev_cleandir can loop forever

OS-5395 zfsdev_ioctl skips secpolicy when FKIOCTL is set
Reviewed by: Robert Mustacchi <rm@joyent.com>

5 files changed, 97 insertions, 33 deletions

diff --git a/usr/src/uts/common/fs/dev/sdev_profile.c b/usr/src/uts/common/fs/dev/sdev_profile.c
index d9cac41258..05a00a21a0 100644
--- a/usr/src/uts/common/fs/dev/sdev_profile.c
+++ b/usr/src/uts/common/fs/dev/sdev_profile.c
@@ -415,7 +415,7 @@ is_nonempty_dir(char *name, char *pathleft, struct sdev_node *dir)
 
 
 /* Check if name passes matching rules */
-static int
+int
 prof_name_matched(char *name, struct sdev_node *dir)
 {
 	int type, match = 0;
diff --git a/usr/src/uts/common/fs/dev/sdev_subr.c b/usr/src/uts/common/fs/dev/sdev_subr.c
index aa9b958d2a..37918bc38f 100644
--- a/usr/src/uts/common/fs/dev/sdev_subr.c
+++ b/usr/src/uts/common/fs/dev/sdev_subr.c
@@ -2148,7 +2148,7 @@ sdev_cleandir(struct sdev_node *ddv, char *expr, uint_t flags)
 	int error = 0;
 	int busy = 0;
 	struct vnode *vp;
-	struct sdev_node *dv;
+	struct sdev_node *dv, *next;
 	int bkstore = 0;
 	int len = 0;
 	char *bks_name = NULL;
@@ -2159,7 +2159,8 @@ sdev_cleandir(struct sdev_node *ddv, char *expr, uint_t flags)
 	 * We try our best to destroy all unused sdev_node's
 	 */
 	rw_enter(&ddv->sdev_contents, RW_WRITER);
-	while ((dv = SDEV_FIRST_ENTRY(ddv)) != NULL) {
+	for (dv = SDEV_FIRST_ENTRY(ddv); dv != NULL; dv = next) {
+		next = SDEV_NEXT_ENTRY(ddv, dv);
 		vp = SDEVTOV(dv);
 
 		if (expr && gmatch(dv->sdev_name, expr) == 0)
diff --git a/usr/src/uts/common/fs/dev/sdev_zvolops.c b/usr/src/uts/common/fs/dev/sdev_zvolops.c
index 5a4b7ad017..5056871dbf 100644
--- a/usr/src/uts/common/fs/dev/sdev_zvolops.c
+++ b/usr/src/uts/common/fs/dev/sdev_zvolops.c
@@ -175,23 +175,25 @@ devzvol_objset_check(char *dsname, dmu_objset_type_t *type)
 	boolean_t	ispool;
 	zfs_cmd_t	*zc;
 	int rc;
+	nvlist_t 	*nvl;
+	size_t nvsz;
 
 	zc = kmem_zalloc(sizeof (zfs_cmd_t), KM_SLEEP);
 	(void) strlcpy(zc->zc_name, dsname, MAXPATHLEN);
 
+	nvl = fnvlist_alloc();
+	fnvlist_add_boolean_value(nvl, "cachedpropsonly", B_TRUE);
+	zc->zc_nvlist_src = (uintptr_t)fnvlist_pack(nvl, &nvsz);
+	zc->zc_nvlist_src_size = nvsz;
+	fnvlist_free(nvl);
+
 	ispool = (strchr(dsname, '/') == NULL) ? B_TRUE : B_FALSE;
-	if (!ispool && sdev_zvol_name2minor(dsname, NULL) == 0) {
-		sdcmn_err13(("found cached minor node"));
-		if (type)
-			*type = DMU_OST_ZVOL;
-		kmem_free(zc, sizeof (zfs_cmd_t));
-		return (0);
-	}
 	rc = devzvol_handle_ioctl(ispool ? ZFS_IOC_POOL_STATS :
 	    ZFS_IOC_OBJSET_STATS, zc, NULL);
 	if (type && rc == 0)
 		*type = (ispool) ? DMU_OST_ZFS :
 		    zc->zc_objset_stats.dds_type;
+	fnvlist_pack_free((char *)(uintptr_t)zc->zc_nvlist_src, nvsz);
 	kmem_free(zc, sizeof (zfs_cmd_t));
 	return (rc);
 }
@@ -260,6 +262,7 @@ devzvol_make_dsname(const char *path, const char *name)
 int
 devzvol_validate(struct sdev_node *dv)
 {
+	vnode_t *vn = SDEVTOV(dv);
 	dmu_objset_type_t do_type;
 	char *dsname;
 	char *nm = dv->sdev_name;
@@ -283,27 +286,55 @@ devzvol_validate(struct sdev_node *dv)
 	if (dsname == NULL)
 		return (SDEV_VTOR_INVALID);
 
+	/*
+	 * Leave any nodes alone that have been explicitly created by
+	 * sdev profiles.
+	 */
+	if (!(dv->sdev_flags & SDEV_GLOBAL) && dv->sdev_origin != NULL) {
+		kmem_free(dsname, strlen(dsname) + 1);
+		return (SDEV_VTOR_VALID);
+	}
+
 	rc = devzvol_objset_check(dsname, &do_type);
 	sdcmn_err13(("  '%s' rc %d", dsname, rc));
 	if (rc != 0) {
-		kmem_free(dsname, strlen(dsname) + 1);
-		return (SDEV_VTOR_INVALID);
+		sdev_node_t *parent = dv->sdev_dotdot;
+		/*
+		 * Explicitly passed-through zvols in our sdev profile can't
+		 * be created as prof_* shadow nodes, because in the GZ they
+		 * are symlinks, but in the NGZ they are actual device files.
+		 *
+		 * The objset_check will fail on these are they are outside
+		 * any delegated dataset (zfs will not allow ioctl access to
+		 * them from this zone). We still want them to work, though.
+		 */
+		if (!(parent->sdev_flags & SDEV_GLOBAL) &&
+		    parent->sdev_origin != NULL &&
+		    !(dv->sdev_flags & SDEV_GLOBAL) &&
+		    (vn->v_type == VBLK || vn->v_type == VCHR) &&
+		    prof_name_matched(nm, parent)) {
+			do_type = DMU_OST_ZVOL;
+		} else {
+			kmem_free(dsname, strlen(dsname) + 1);
+			return (SDEV_VTOR_INVALID);
+		}
 	}
+
 	sdcmn_err13(("  v_type %d do_type %d",
-	    SDEVTOV(dv)->v_type, do_type));
-	if ((SDEVTOV(dv)->v_type == VLNK && do_type != DMU_OST_ZVOL) ||
-	    ((SDEVTOV(dv)->v_type == VBLK || SDEVTOV(dv)->v_type == VCHR) &&
+	    vn->v_type, do_type));
+	if ((vn->v_type == VLNK && do_type != DMU_OST_ZVOL) ||
+	    ((vn->v_type == VBLK || vn->v_type == VCHR) &&
 	    do_type != DMU_OST_ZVOL) ||
-	    (SDEVTOV(dv)->v_type == VDIR && do_type == DMU_OST_ZVOL)) {
+	    (vn->v_type == VDIR && do_type == DMU_OST_ZVOL)) {
 		kmem_free(dsname, strlen(dsname) + 1);
 		return (SDEV_VTOR_STALE);
 	}
-	if (SDEVTOV(dv)->v_type == VLNK) {
+	if (vn->v_type == VLNK) {
 		char *ptr, *link;
 		long val = 0;
 		minor_t lminor, ominor;
 
-		rc = sdev_getlink(SDEVTOV(dv), &link);
+		rc = sdev_getlink(vn, &link);
 		ASSERT(rc == 0);
 
 		ptr = strrchr(link, ':') + 1;
@@ -582,9 +613,29 @@ devzvol_mk_ngz_node(struct sdev_node *parent, char *nm)
 		return (ENOENT);
 
 	if (devzvol_objset_check(dsname, &do_type) != 0) {
-		kmem_free(dsname, strlen(dsname) + 1);
-		return (ENOENT);
+		/*
+		 * objset_check will succeed on any valid objset in the global
+		 * zone, and any valid delegated dataset. It will fail, however,
+		 * in non-global zones on explicitly whitelisted zvol devices
+		 * that are outside any delegated dataset.
+		 *
+		 * The directories leading up to the zvol device itself will be
+		 * created by prof for us in advance (and will always validate
+		 * because of the matching check in devzvol_validate). The zvol
+		 * device itself can't be created by prof though because in the
+		 * GZ it's a symlink, and in the NGZ it is not. So, we create
+		 * such zvol device files here.
+		 */
+		if (!(parent->sdev_flags & SDEV_GLOBAL) &&
+		    parent->sdev_origin != NULL &&
+		    prof_name_matched(nm, parent)) {
+			do_type = DMU_OST_ZVOL;
+		} else {
+			kmem_free(dsname, strlen(dsname) + 1);
+			return (ENOENT);
+		}
 	}
+
 	if (do_type == DMU_OST_ZVOL)
 		expected_type = VBLK;
 
@@ -645,18 +696,7 @@ devzvol_lookup(struct vnode *dvp, char *nm, struct vnode **vpp,
 		return (error);
 
 	rw_enter(&parent->sdev_contents, RW_READER);
-	if (SDEV_IS_GLOBAL(parent)) {
-		/*
-		 * During iter_datasets, don't create GZ dev when running in
-		 * NGZ.  We can't return ENOENT here since that could
-		 * incorrectly trigger the creation of the dev from the
-		 * recursive call through prof_filldir during iter_datasets.
-		 */
-		if (getzoneid() != GLOBAL_ZONEID) {
-			rw_exit(&parent->sdev_contents);
-			return (EPERM);
-		}
-	} else {
+	if (!SDEV_IS_GLOBAL(parent)) {
 		int res;
 
 		rw_exit(&parent->sdev_contents);
@@ -693,6 +733,28 @@ devzvol_lookup(struct vnode *dvp, char *nm, struct vnode **vpp,
 		return (res);
 	}
 
+	/*
+	 * Don't let the global-zone style lookup succeed here when we're not
+	 * running in the global zone. This can happen because prof calls into
+	 * us (in prof_filldir) trying to create an explicitly passed-through
+	 * zvol device outside any delegated dataset.
+	 *
+	 * We have to stop this here or else we will create prof shadows of
+	 * the global zone symlink, which will make no sense at all in the
+	 * non-global zone (it has no /devices for the symlink to point at).
+	 *
+	 * These zvols will be created later (at access time) by mk_ngz_node
+	 * instead. The dirs leading up to them will be created by prof
+	 * internally.
+	 *
+	 * We have to return EPERM here, because ENOENT is given special
+	 * meaning by prof in this context.
+	 */
+	if (getzoneid() != GLOBAL_ZONEID) {
+		rw_exit(&parent->sdev_contents);
+		return (EPERM);
+	}
+
 	dsname = devzvol_make_dsname(parent->sdev_path, nm);
 	rw_exit(&parent->sdev_contents);
 	sdcmn_err13(("rvp dsname %s", dsname ? dsname : "(null)"));
diff --git a/usr/src/uts/common/fs/zfs/zfs_ioctl.c b/usr/src/uts/common/fs/zfs/zfs_ioctl.c
index 7d9947b870..ec79fc5cd3 100644
--- a/usr/src/uts/common/fs/zfs/zfs_ioctl.c
+++ b/usr/src/uts/common/fs/zfs/zfs_ioctl.c
@@ -6060,7 +6060,7 @@ zfsdev_ioctl(dev_t dev, int cmd, intptr_t arg, int flag, cred_t *cr, int *rvalp)
 	}
 
 
-	if (error == 0 && !(flag & FKIOCTL))
+	if (error == 0)
 		error = vec->zvec_secpolicy(zc, innvl, cr);
 
 	if (error != 0)
diff --git a/usr/src/uts/common/sys/fs/sdev_impl.h b/usr/src/uts/common/sys/fs/sdev_impl.h
index 644fda0482..90336efa9c 100644
--- a/usr/src/uts/common/sys/fs/sdev_impl.h
+++ b/usr/src/uts/common/sys/fs/sdev_impl.h
@@ -488,6 +488,7 @@ extern int sdev_copyin_mountargs(struct mounta *, struct sdev_mountargs *);
 extern int sdev_reserve_subdirs(struct sdev_node *);
 extern int prof_lookup();
 extern void prof_filldir(struct sdev_node *);
+extern int prof_name_matched(char *, struct sdev_node *);
 extern int devpts_validate(struct sdev_node *dv);
 extern int devnet_validate(struct sdev_node *dv);
 extern int devipnet_validate(struct sdev_node *dv);