Date: Wed, 11 Feb 2004 20:57:34 -0700 From: Scott Long <scottl@freebsd.org> To: Andre Guibert de Bruet <andy@siliconlandmark.com> Cc: scsi@freebsd.org Subject: Re: make_dev(9) perms for SCSI & SCSI RAID drivers in CURRENT. (fwd) Message-ID: <402AF9AE.20302@freebsd.org> In-Reply-To: <20040211212115.G91658@alpha.siliconlandmark.com> References: <20040211212115.G91658@alpha.siliconlandmark.com>
next in thread | previous in thread | raw e-mail | index | archive | help
I'll tackle some of these in the next few days. I'm focused on releasing 5.2.1 at the moment. Scott Andre Guibert de Bruet wrote: > 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@siliconlandmark.com> > To: Bruce Evans <bde@zeta.org.au> > Cc: current@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/ > >> >> >>------------------------------------------------------------------------ >> >>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; >> >> >>------------------------------------------------------------------------ >> >>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; >> >> >> >>------------------------------------------------------------------------ >> >>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 */ >> >> >> >>------------------------------------------------------------------------ >> >>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; >> >> >> >>------------------------------------------------------------------------ >> >>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; >> >> >> >>------------------------------------------------------------------------ >> >>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); >> >> >>------------------------------------------------------------------------ >> >>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); >> >> >>------------------------------------------------------------------------ >> >>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; >> >> >>------------------------------------------------------------------------ >> >>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; >> >> >> >>------------------------------------------------------------------------ >> >>_______________________________________________ >>freebsd-current@freebsd.org mailing list >>http://lists.freebsd.org/mailman/listinfo/freebsd-current >>To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?402AF9AE.20302>