make_dev(9) perms for SCSI & SCSI RAID drivers in CURRENT. (fwd)
Scott Long
scottl at freebsd.org
Wed Feb 11 20:00:10 PST 2004
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 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/ >
>>
>>
>>------------------------------------------------------------------------
>>
>>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 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