Date: Tue, 30 Nov 2004 01:48:37 -0500 From: Craig Rodrigues <rodrigc@crodrigues.org> To: freebsd-usb@freebsd.org Subject: Re: Changing permissions of /dev/usb[n] to 664? Message-ID: <20041130064837.GA1541@crodrigues.org> In-Reply-To: <20041107.125814.34760598.imp@bsdimp.com> References: <20041107064227.GA79915@crodrigues.org> <20041107.125814.34760598.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Nov 07, 2004 at 12:58:14PM -0700, M. Warner Losh wrote: > This looks good, but we should audit all the ioctls to make sure the > ones that modify anything have the proper checks to make sure the fd > was opened for write. OK. Here is another iteration of the patch. It does the following: - opens /dev/usb[n] as 664 - puts suser() permission checks in the following paths: USB_REQUEST ioctl() usbpoll() usbread() This is what a non-root user can and cannot do on /dev/usb[n]: Allowed ======= USB_DISCOVER USB_DEVICEINFO USB_DEVICESTATS usbopen() usbclose() Forbidden ========= USB_REQUEST usbread() usbpoll() The result of this patch is that a non-root user can run usbdevs without a problem. I also have a small test program where I tried running different ioctl's as non-root and this is the output I got: Executing ioctl(): USB_REQUEST Operation not permitted Executing ioctl(): USB_DISCOVER...OK Executing ioctl(): USB_DEVICEINFO...OK Executing ioctl(): USB_DEVICESTATS...OK Comments? -- Craig Rodrigues rodrigc@crodrigues.org --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="usb.c.patch.txt" --- usb.c.orig Mon Nov 29 23:27:20 2004 +++ usb.c Tue Nov 30 00:04:00 2004 @@ -320,11 +320,11 @@ /* The per controller devices (used for usb_discover) */ /* XXX This is redundant now, but old usbd's will want it */ sc->sc_usbdev = make_dev(&usb_cdevsw, device_get_unit(self), UID_ROOT, - GID_OPERATOR, 0660, "usb%d", device_get_unit(self)); + GID_OPERATOR, 0664, "usb%d", device_get_unit(self)); if (usb_ndevs++ == 0) { /* The device spitting out events */ usb_dev = make_dev(&usb_cdevsw, USB_DEV_MINOR, UID_ROOT, - GID_OPERATOR, 0660, "usb"); + GID_OPERATOR, 0664, "usb"); } #endif @@ -515,13 +515,16 @@ int unit = USBUNIT(dev); int s, error, n; + error = suser(curthread); + if (error) + return error; + if (unit != USB_DEV_MINOR) return (ENODEV); if (uio->uio_resid != sizeof(struct usb_event)) return (EINVAL); - error = 0; s = splusb(); for (;;) { n = usb_get_next_event(&ue); @@ -605,6 +608,10 @@ usbd_status err; int error = 0; + error = suser(p); + if (error) + return error; + DPRINTF(("usbioctl: USB_REQUEST addr=%d len=%d\n", addr, len)); if (len < 0 || len > 32768) return (EINVAL); @@ -680,6 +687,10 @@ { int revents, mask, s; int unit = USBUNIT(dev); + int error = suser(p); + + if (error) + return error; if (unit == USB_DEV_MINOR) { revents = 0; --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="usbtest.c" #include <stdio.h> #include <stdlib.h> #include <fcntl.h> #include <sys/ioctl.h> #include <dev/usb/usb.h> #include <errno.h> void execute_ioctl(int fd, int cmd, void *arg, const char *name) { int e; printf("Executing ioctl(): %s", name); e = ioctl(fd, cmd, arg); if (e == -1 ) { printf(" %s\n", strerror(errno)); } else { printf("...OK\n"); } } int main(int argc, char *argv[]) { struct usb_ctl_request ucr; struct usb_device_stats uds; struct usb_device_info udi; int fd = open("/dev/usb1", O_RDONLY); if (fd == -1 ) { perror(":"); exit(1); } ucr.ucr_request.wLength[0] = 0x00; ucr.ucr_request.wLength[1] = 0x80; udi.udi_addr = 1; execute_ioctl(fd, USB_REQUEST, &ucr, "USB_REQUEST"); execute_ioctl(fd, USB_DISCOVER, NULL, "USB_DISCOVER"); execute_ioctl(fd, USB_DEVICEINFO, &udi, "USB_DEVICEINFO"); execute_ioctl(fd, USB_DEVICESTATS, &uds, "USB_DEVICESTATS"); close(fd); return 0; } --Q68bSM7Ycu6FN28Q--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041130064837.GA1541>