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. Message-ID: <20040208151037.J91658@alpha.siliconlandmark.com> In-Reply-To: <20040208235936.M44718@gamplex.bde.org> References: <20040208022417.M91658@alpha.siliconlandmark.com> <20040208235936.M44718@gamplex.bde.org>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] 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/ > [-- Attachment #2 --] 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; [-- Attachment #3 --] 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; [-- Attachment #4 --] 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 */ [-- Attachment #5 --] 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; [-- Attachment #6 --] 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; [-- Attachment #7 --] 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); [-- Attachment #8 --] 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); [-- Attachment #9 --] 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; [-- Attachment #10 --] 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;help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040208151037.J91658>