From owner-freebsd-usb@FreeBSD.ORG Tue Nov 30 06:48:43 2004 Return-Path: Delivered-To: freebsd-usb@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7BC7816A4CE for ; Tue, 30 Nov 2004 06:48:43 +0000 (GMT) Received: from rwcrmhc13.comcast.net (rwcrmhc13.comcast.net [204.127.198.39]) by mx1.FreeBSD.org (Postfix) with ESMTP id 40FC443D4C for ; Tue, 30 Nov 2004 06:48:43 +0000 (GMT) (envelope-from rodrigc@crodrigues.org) Received: from h00609772adf0.ne.client2.attbi.com ([66.30.114.143]) by comcast.net (rwcrmhc13) with ESMTP id <2004113006483901500c8vlhe>; Tue, 30 Nov 2004 06:48:40 +0000 Received: from h00609772adf0.ne.client2.attbi.com (localhost [127.0.0.1]) iAU6mcHQ001613; Tue, 30 Nov 2004 01:48:38 -0500 (EST) (envelope-from rodrigc@h00609772adf0.ne.client2.attbi.com) Received: (from rodrigc@localhost)iAU6mbE7001612; Tue, 30 Nov 2004 01:48:37 -0500 (EST) (envelope-from rodrigc) Date: Tue, 30 Nov 2004 01:48:37 -0500 From: Craig Rodrigues To: freebsd-usb@freebsd.org Message-ID: <20041130064837.GA1541@crodrigues.org> References: <20041107064227.GA79915@crodrigues.org> <20041107.125814.34760598.imp@bsdimp.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Q68bSM7Ycu6FN28Q" Content-Disposition: inline In-Reply-To: <20041107.125814.34760598.imp@bsdimp.com> User-Agent: Mutt/1.4.1i Subject: Re: Changing permissions of /dev/usb[n] to 664? X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Nov 2004 06:48:43 -0000 --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 #include #include #include #include #include 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--