Patch to convert usb2 to use cdev
Hans Petter Selasky
hselasky at c2i.net
Sun Nov 9 07:13:20 PST 2008
After going through your patch I have a feeling you quite well understand how
USB2 is working with regard to the file system.
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 , 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.
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 
> 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
> Feel free to review this patch; I'd like to commit it to HEAD at the end
> of the week or so.
>  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