OS-6558 sdev_create_minor_node should allow non-666

Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Jerry Jelinek <jerry.jelinek@joyent.com>

2 files changed, 36 insertions, 7 deletions

diff --git a/usr/src/uts/common/fs/dev/sdev_plugin.c b/usr/src/uts/common/fs/dev/sdev_plugin.c
index bca7325bfb..4f862ccb72 100644
--- a/usr/src/uts/common/fs/dev/sdev_plugin.c
+++ b/usr/src/uts/common/fs/dev/sdev_plugin.c
@@ -248,28 +248,50 @@ sdev_plugin_mknod(sdev_ctx_t ctx, char *name, mode_t mode, dev_t dev)
 	sdev_node_t *sdvp;
 	timestruc_t now;
 	struct vattr vap;
+	mode_t type = mode & S_IFMT;
+	mode_t access = mode & S_IAMB;
 
 	if (sdev_plugin_name_isvalid(name, SDEV_PLUGIN_NAMELEN) == 0)
 		return (EINVAL);
 
 	sdvp = (sdev_node_t *)ctx;
 	ASSERT(RW_WRITE_HELD(&sdvp->sdev_contents));
-	if (mode != S_IFCHR && mode != S_IFBLK)
+
+	/*
+	 * Ensure only type and user/group/other permission bits are present.
+	 * Do not allow setuid, setgid, etc.
+	 */
+	if ((mode & ~(S_IFMT | S_IAMB)) != 0)
+		return (EINVAL);
+
+	/* Disallow types other than character and block devices */
+	if (type != S_IFCHR && type != S_IFBLK)
 		return (EINVAL);
 
+	/* Disallow execute bits */
+	if ((access & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0)
+		return (EINVAL);
+
+	/* No bits other than 0666 in access */
+	ASSERT((access &
+	    ~(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)) == 0);
+
+	/* Default to relatively safe access bits if none specified. */
+	if (access == 0)
+		access = 0600;
+
 	ASSERT(sdvp->sdev_private != NULL);
 
-	vap = *sdev_getdefault_attr(mode == S_IFCHR ? VCHR : VBLK);
+	vap = *sdev_getdefault_attr(type == S_IFCHR ? VCHR : VBLK);
 	gethrestime(&now);
 	vap.va_atime = now;
 	vap.va_mtime = now;
 	vap.va_ctime = now;
 	vap.va_rdev = dev;
-	vap.va_mode = mode | 0666;
+	vap.va_mode = type | access;
 
 	/* Despite the similar name, this is in fact a different function */
 	return (sdev_plugin_mknode(sdvp->sdev_private, sdvp, name, &vap));
-
 }
 
 static int
diff --git a/usr/src/uts/common/io/vnd/vnd.c b/usr/src/uts/common/io/vnd/vnd.c
index 5766b39692..5a25ed22d5 100644
--- a/usr/src/uts/common/io/vnd/vnd.c
+++ b/usr/src/uts/common/io/vnd/vnd.c
@@ -950,6 +950,13 @@ size_t vnd_flush_nburst = 10;			/* 10 frames */
 #define	VND_SDEV_ZROOT	"/dev/vnd/zone"
 
 /*
+ * vnd relies on privileges, not mode bits to limit access.  As such, device
+ * files are read-write to everyone.
+ */
+#define	VND_SDEV_MODE	(S_IFCHR | S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | \
+			S_IROTH | S_IWOTH)
+
+/*
  * Statistic macros
  */
 #define	VND_STAT_INC(vsp, field, val) \
@@ -5420,8 +5427,8 @@ vnd_sdev_fillzone(vnd_pnsd_t *nsp, sdev_ctx_t ctx)
 		mutex_enter(&vdp->vdd_lock);
 		if ((vdp->vdd_flags & VND_D_LINKED) &&
 		    !(vdp->vdd_flags & (VND_D_CONDEMNED | VND_D_ZONE_DYING))) {
-			ret = sdev_plugin_mknod(ctx, vdp->vdd_lname, S_IFCHR,
-			    vdp->vdd_devid);
+			ret = sdev_plugin_mknod(ctx, vdp->vdd_lname,
+			    VND_SDEV_MODE, vdp->vdd_devid);
 			if (ret != 0 && ret != EEXIST) {
 				mutex_exit(&vdp->vdd_lock);
 				mutex_exit(&nsp->vpnd_lock);
@@ -5465,7 +5472,7 @@ vnd_sdev_filldir_root(sdev_ctx_t ctx)
 	 * Always add a reference to the control node. There's no need to
 	 * reference it since it always exists and is always what we clone from.
 	 */
-	ret = sdev_plugin_mknod(ctx, "ctl", S_IFCHR,
+	ret = sdev_plugin_mknod(ctx, "ctl", VND_SDEV_MODE,
 	    makedevice(ddi_driver_major(vnd_dip), 0));
 	if (ret != 0 && ret != EEXIST)
 		return (ret);