Date: Sun, 9 Nov 2008 15:15:25 +0100 From: Hans Petter Selasky <hselasky@c2i.net> To: freebsd-usb@freebsd.org Cc: usb@freebsd.org Subject: Re: Patch to convert usb2 to use cdev Message-ID: <200811091515.26123.hselasky@c2i.net> In-Reply-To: <20081109120257.GA78524@rink.nu> References: <20081109120257.GA78524@rink.nu>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Rick, After going through your patch I have a feeling you quite well understand how USB2 is working with regard to the file system. Some issues: 1) You don't have to create an alias in "usb2_fifo_attach". The alias /dev/usbX.Y.Z.T is mostly for internal usage. + make_dev_alias(f_sc->dev, buf); 2) struct usb2_privdata I would call the structure "usb2_fs_privdata" so that it is clear that this is File-System related private data. There is also a field called "xfer->priv_sc" so it might be confusing? 3) You need to solve the problem about a per-open-call context for /dev/ugenX.Y . This device is supposed to be cloneable, that means multiple processes are allowed to open it and establish independant connections to the USB stack. Here are also some tricky issues with permissions, because I allow trunking of multiple endpoints through the same file-handle, called USB FS, and you have to verify that the current thread has permission to open the endpoint inside an ioctl function. 4) You need to generate dummy /dev/ugenX.Y.0 ... 15 inclusivly, endpoint holders. Typically there are not 15 endpoints, but it is difficult to in-advance figure out this number. 5) Given that you use "destroy_dev_sched_cb" it becomes very easy to end up in a situation with multiple cdev instances having identical names, because the "destroy_dev_sched_cb" does not delete the device until the process which has the device opened closes it. Especially when re-attaching an USB device. Regarding your finding [1], I've assumed that lookup and open of a file is atomic in devfs regard. Else you would have to change the devfs-clone interface to be able to solve the problem passing along the global variables. --HPS On Sunday 09 November 2008, Rink Springer wrote: > Hi everyone, > > I've made a patch, available at http://rink.nu/tmp/usb-cdev.diff, which > removes the custom "/dev/usb " device, associated event handlers and > custom ownership/permissions structures and converts the whole deal to > use make_dev(9) and friends. The end result is that every USB device > will get a /dev entry, which can be chmod(1)-ed, chown(1)-ed etc as > usual - futhermore, possible races between looking up a device name and > opening it are completely removed by this patch [1] > > usbconfig(8) works as before after applying the patch, but obviously, > commands that involve setting permissions or ownership will return an > error as those ioctl's are no longer present; I intend to remove them > completely and from usbconfig itself after this patch has been > committed. > > Feel free to review this patch; I'd like to commit it to HEAD at the end > of the week or so. > > [1] The previous code would set a global variable to determine which > USB device corresponds with the file being looked up, and a > subsequent open call would open this device. I don't know the VFS > well enough to determine if this can be exploited, but it doesn't > look right to me :-) >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200811091515.26123.hselasky>