Patch to convert usb2 to use cdev

Hans Petter Selasky hselasky at c2i.net
Sun Nov 9 07:13:20 PST 2008


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 :-)
>


More information about the freebsd-usb mailing list