make_dev(9) perms for SCSI & SCSI RAID drivers in CURRENT. (fwd)

Andre Guibert de Bruet andy at siliconlandmark.com
Wed Feb 11 18:52:17 PST 2004


Gentlemen,

Do you have any objections to the attached patches? If not, could you
commit them? Thanks. :-)

Regards,
Andy

PS: Please copy me in any responses to freebsd-scsi as I am not on the
list. Thanks.

> Andre Guibert de Bruet | Enterprise Software Consultant >
> Silicon Landmark, LLC. | http://siliconlandmark.com/    >

---------- Forwarded message ----------
Date: Sun, 8 Feb 2004 15:34:08 -0500 (EST)
From: Andre Guibert de Bruet <andy at siliconlandmark.com>
To: Bruce Evans <bde at zeta.org.au>
Cc: current at freebsd.org
Subject: Re: make_dev(9) perms for SCSI & SCSI RAID drivers in CURRENT.


On Mon, 9 Feb 2004, Bruce Evans wrote:

> On Sun, 8 Feb 2004, Andre Guibert de Bruet wrote:
>
> > While studying the various FreeBSD SCSI and SCSI RAID drivers, I noticed
> > that the file mode (perm mask) varies per driver. So far, I've come across
> > 0600, 0640 and 0644. I can't really see why any of these drivers would
> > have anything other than 0600, as it would require root access or at least
> > write perm to do anything useful with the card.
>
> All disk (data) devices should have mode 0640 and ownership root:operator
> and all disk (control) devices should have mode 0600 and ownership root:wheel.
> Distributed setting of ownerships and permissions gives many more bugs than
> centralized setting in MAKEDEV.  Mode bugs in devfs start at its top level
> (its directory has mode 555 although its owner can write to it except
> possibly in the jailed case).
>
> > Here's a quick illustration of what I'm refering to:
> >
> > aac	0640 (octal notation in code)
> > amr	0600 (implemented as S_IRUSR | S_IWUSR)
> > asr	0640 (octal notation in code)
> > ciss	0600 (implemented as S_IRUSR | S_IWUSR)
> > ida	0600 (implemented as S_IRUSR | S_IWUSR)
> > iir	0644 (implemented as S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
> > ips	0600 (implemented as S_IRUSR | S_IWUSR)
> > isp	0600 (octal notation in code)
> > mly	0600 (implemented as S_IRUSR | S_IWUSR)
>
> Most of these actually create control devices, so mode 0600 is correct
> and group operator is bogus, and mode 0640 is a potental security hole
> especially with group operator.  Group operator is almost always used
> of course.  The data devices are mostly created by the disk mini-layer
> in RELENG_4 (except RELENG_4 doesn't really have devfs) and by GEOM in
> -current.

I adjusted and expanded the set of patches that I had to change
permissions on the control devices so that they also set the GID to wheel.
The assumption that I am making with these patches is that the drivers
that are calling make_dev() are creating control devices, as they should
be letting GEOM create their data devices. Feedback is welcome here as my
GEOM-fu isn't all that hot...

I have tried to maintain the style used in the drivers themselves and
fixed the long line in the patch for isp_freebsd.c.

Regards,
Andy

> Andre Guibert de Bruet | Enterprise Software Consultant >
> Silicon Landmark, LLC. | http://siliconlandmark.com/    >
-------------- next part --------------
Index: aac.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/aac/aac.c,v
retrieving revision 1.85
diff -u -r1.85 aac.c
--- aac.c	7 Feb 2004 17:40:37 -0000	1.85
+++ aac.c	8 Feb 2004 19:39:26 -0000
@@ -51,6 +51,7 @@
 #include <sys/signalvar.h>
 #include <sys/time.h>
 #include <sys/eventhandler.h>
+#include <sys/stat.h>
 
 #include <machine/bus_memio.h>
 #include <machine/bus.h>
@@ -270,8 +271,8 @@
 	 * Make the control device.
 	 */
 	unit = device_get_unit(sc->aac_dev);
-	sc->aac_dev_t = make_dev(&aac_cdevsw, unit, UID_ROOT, GID_OPERATOR,
-				 0640, "aac%d", unit);
+	sc->aac_dev_t = make_dev(&aac_cdevsw, unit, UID_ROOT, GID_WHEEL,
+				 S_IRUSR | S_IWUSR, "aac%d", unit);
 	(void)make_dev_alias(sc->aac_dev_t, "afa%d", unit);
 	(void)make_dev_alias(sc->aac_dev_t, "hpn%d", unit);
 	sc->aac_dev_t->si_drv1 = sc;
-------------- next part --------------
Index: amr.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/amr/amr.c,v
retrieving revision 1.50
diff -u -r1.50 amr.c
--- amr.c	8 Feb 2004 16:07:22 -0000	1.50
+++ amr.c	8 Feb 2004 19:54:05 -0000
@@ -236,7 +236,7 @@
     /*
      * Create the control device.
      */
-    sc->amr_dev_t = make_dev(&amr_cdevsw, device_get_unit(sc->amr_dev), UID_ROOT, GID_OPERATOR,
+    sc->amr_dev_t = make_dev(&amr_cdevsw, device_get_unit(sc->amr_dev), UID_ROOT, GID_WHEEL,
 			     S_IRUSR | S_IWUSR, "amr%d", device_get_unit(sc->amr_dev));
     sc->amr_dev_t->si_drv1 = sc;
 
-------------- next part --------------
Index: asr.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/asr/asr.c,v
retrieving revision 1.38
diff -u -r1.38 asr.c
--- asr.c	26 Sep 2003 15:56:42 -0000	1.38
+++ asr.c	8 Feb 2004 19:40:19 -0000
@@ -3127,8 +3127,8 @@
 	/*
 	 *	Generate the device node information
 	 */
-	(void)make_dev(&asr_cdevsw, unit, UID_ROOT, GID_OPERATOR, 0640,
-	    "rasr%d", unit);
+	(void)make_dev(&asr_cdevsw, unit, UID_ROOT, GID_WHEEL,
+	    S_IRUSR | S_IWUSR, "rasr%d", unit);
 	ATTACH_RETURN(0);
 } /* asr_attach */
 
-------------- next part --------------
Index: ciss.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ciss/ciss.c,v
retrieving revision 1.34
diff -u -r1.34 ciss.c
--- ciss.c	18 Jan 2004 16:55:01 -0000	1.34
+++ ciss.c	8 Feb 2004 20:02:28 -0000
@@ -403,7 +403,7 @@
      * Create the control device.
      */
     sc->ciss_dev_t = make_dev(&ciss_cdevsw, device_get_unit(sc->ciss_dev),
-			      UID_ROOT, GID_OPERATOR, S_IRUSR | S_IWUSR,
+			      UID_ROOT, GID_WHEEL, S_IRUSR | S_IWUSR,
 			      "ciss%d", device_get_unit(sc->ciss_dev));
     sc->ciss_dev_t->si_drv1 = sc;
 
-------------- next part --------------
Index: ida.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ida/ida.c,v
retrieving revision 1.34
diff -u -r1.34 ida.c
--- ida.c	15 Jan 2004 06:37:52 -0000	1.34
+++ ida.c	8 Feb 2004 20:03:54 -0000
@@ -277,7 +277,7 @@
 	}
 
 	ida->ida_dev_t = make_dev(&ida_cdevsw, ida->unit,
-				 UID_ROOT, GID_OPERATOR, S_IRUSR | S_IWUSR,
+				 UID_ROOT, GID_WHEEL, S_IRUSR | S_IWUSR,
 				 "ida%d", ida->unit);
 	ida->ida_dev_t->si_drv1 = ida;
 
-------------- next part --------------
Index: iir_ctrl.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/iir/iir_ctrl.c,v
retrieving revision 1.11
diff -u -r1.11 iir_ctrl.c
--- iir_ctrl.c	26 Sep 2003 15:36:47 -0000	1.11
+++ iir_ctrl.c	8 Feb 2004 19:50:00 -0000
@@ -102,13 +102,13 @@
     dev_t dev;
 
 #ifdef SDEV_PER_HBA
-    dev = make_dev(&iir_cdevsw, hba2minor(unit), UID_ROOT, GID_OPERATOR,
-                   S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, "iir%d", unit);
+    dev = make_dev(&iir_cdevsw, hba2minor(unit), UID_ROOT, GID_WHEEL,
+                   S_IRUSR | S_IWUSR, "iir%d", unit);
 #else
     if (sdev_made)
         return (0);
-    dev = make_dev(&iir_cdevsw, 0, UID_ROOT, GID_OPERATOR,
-                   S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, "iir");
+    dev = make_dev(&iir_cdevsw, 0, UID_ROOT, GID_WHEEL,
+                   S_IRUSR | S_IWUSR, "iir");
     sdev_made = 1;
 #endif
     return (dev);
-------------- next part --------------
Index: ips.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ips/ips.c,v
retrieving revision 1.7
diff -u -r1.7 ips.c
--- ips.c	18 Jan 2004 17:34:11 -0000	1.7
+++ ips.c	8 Feb 2004 20:05:58 -0000
@@ -451,7 +451,7 @@
 		device_printf(sc->dev, "failed to initialize command buffers\n");
 		goto error;
 	}
-        sc->device_file = make_dev(&ips_cdevsw, device_get_unit(sc->dev), UID_ROOT, GID_OPERATOR,
+        sc->device_file = make_dev(&ips_cdevsw, device_get_unit(sc->dev), UID_ROOT, GID_WHEEL,
                                         S_IRUSR | S_IWUSR, "ips%d", device_get_unit(sc->dev));
 	sc->device_file->si_drv1 = sc;
 	ips_diskdev_init(sc);
-------------- next part --------------
Index: isp_freebsd.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/isp/isp_freebsd.c,v
retrieving revision 1.97
diff -u -r1.97 isp_freebsd.c
--- isp_freebsd.c	8 Feb 2004 19:17:56 -0000	1.97
+++ isp_freebsd.c	8 Feb 2004 20:00:36 -0000
@@ -35,6 +35,7 @@
 #include <sys/conf.h>
 #include <sys/module.h>
 #include <sys/ioccom.h>
+#include <sys/stat.h>
 #include <dev/isp/isp_ioctl.h>
 
 
@@ -205,7 +206,8 @@
 	 * Create device nodes
 	 */
 	(void) make_dev(&isp_cdevsw, device_get_unit(isp->isp_dev), UID_ROOT,
-	    GID_OPERATOR, 0600, "%s", device_get_nameunit(isp->isp_dev));
+	    GID_WHEEL, S_IRUSR | S_IWUSR, "%s",
+	    device_get_nameunit(isp->isp_dev));
 
 	if (isp->isp_role != ISP_ROLE_NONE) {
 		isp->isp_state = ISP_RUNSTATE;
-------------- next part --------------
Index: mly.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/mly/mly.c,v
retrieving revision 1.32
diff -u -r1.32 mly.c
--- mly.c	18 Jan 2004 12:49:36 -0000	1.32
+++ mly.c	8 Feb 2004 20:08:43 -0000
@@ -316,7 +316,7 @@
     /*
      * Create the control device.
      */
-    sc->mly_dev_t = make_dev(&mly_cdevsw, device_get_unit(sc->mly_dev), UID_ROOT, GID_OPERATOR,
+    sc->mly_dev_t = make_dev(&mly_cdevsw, device_get_unit(sc->mly_dev), UID_ROOT, GID_WHEEL,
 			     S_IRUSR | S_IWUSR, "mly%d", device_get_unit(sc->mly_dev));
     sc->mly_dev_t->si_drv1 = sc;
 
-------------- next part --------------
_______________________________________________
freebsd-current at freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"


More information about the freebsd-scsi mailing list