On Wednesday 22 August 2012 13:42:47 Hans de Goede wrote:
> Apps which deal with devices which also have a kernel driver, need to do
> the following:
> 1) Check which driver is attached, so as to not detach the wrong driver
>    (ie detaching usbfs while another instance of the app is using the device)
> 2) Detach the kernel driver
> 3) Claim the interface
> 
> Where moving from one step to the next for both 1-2 and 2-3 consists of
> a (small) race window. So currently such apps are racy and people just live
> with it.
> 
> This patch adds a new ioctl which makes it possible for apps to do this
> in a race free manner. For flexibility apps can choose to:
> 1) Specify the driver to disconnect
> 2) Specify to disconnect any driver except for the one named by the app
> 3) Disconnect any driver
> 
> Note that if there is no driver attached, the ioctl will just act like the
> regular claim-interface ioctl, this is by design, as returning an error for
> this condition would open a new bag of race-conditions.
> 
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
>  drivers/usb/core/devio.c     | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/usbdevice_fs.h | 14 ++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index ebb8a9d..829edce 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1928,6 +1928,38 @@ static int proc_get_capabilities(struct dev_state *ps, 
> void __user *arg)
>       return 0;
>  }
>  
> +static int proc_disconnect_claim(struct dev_state *ps, void __user *arg)
> +{
> +     struct usbdevfs_disconnect_claim dc;
> +     struct usb_interface *intf;
> +
> +     if (copy_from_user(&dc, arg, sizeof(dc)))
> +             return -EFAULT;
> +
> +     intf = usb_ifnum_to_if(ps->dev, dc.interface);
> +     if (!intf)
> +             return -EINVAL;
> +
> +     if (intf->dev.driver) {
> +             struct usb_driver *driver = to_usb_driver(intf->dev.driver);
> +
> +             if ((dc.flags & USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER) &&
> +                     strncmp(dc.driver, intf->dev.driver->name,
> +                             sizeof(dc.driver)) != 0)

You have no idea what is in the memory behind dev.driver->name which you
will happily compare to and thus access. Potentially you violate the DMA 
coherency
rules here.

> +                     return -EBUSY;
> +
> +             if ((dc.flags & USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER) &&
> +                     strncmp(dc.driver, intf->dev.driver->name,
> +                             sizeof(dc.driver)) == 0)
> +                     return -EBUSY;

Both flags could be set. You should catch that case.

> +
> +             dev_dbg(&intf->dev, "disconnect by usbfs\n");
> +             usb_driver_release_interface(driver, intf);
> +     }
> +
> +     return claimintf(ps, dc.interface);

So you may return an error and yet execute a part of the command.

> +}
> +
>  /*
>   * NOTE:  All requests here that have interface numbers as parameters
>   * are assuming that somehow the configuration has been prevented from
> @@ -2101,6 +2133,9 @@ static long usbdev_do_ioctl(struct file *file, unsigned 
> int cmd,
>       case USBDEVFS_GET_CAPABILITIES:
>               ret = proc_get_capabilities(ps, p);
>               break;
> +     case USBDEVFS_DISCONNECT_CLAIM:
> +             ret = proc_disconnect_claim(ps, p);
> +             break;
>       }
>       usb_unlock_device(dev);
>       if (ret >= 0)
> diff --git a/include/linux/usbdevice_fs.h b/include/linux/usbdevice_fs.h
> index 3b74666..4abe28e 100644
> --- a/include/linux/usbdevice_fs.h
> +++ b/include/linux/usbdevice_fs.h
> @@ -131,6 +131,19 @@ struct usbdevfs_hub_portinfo {
>  #define USBDEVFS_CAP_NO_PACKET_SIZE_LIM              0x04
>  #define USBDEVFS_CAP_BULK_SCATTER_GATHER     0x08
>  
> +/* USBDEVFS_DISCONNECT_CLAIM flags & struct */
> +
> +/* disconnect-and-claim if the driver matches the driver field */
> +#define USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER  0x01
> +/* disconnect-and-claim except when the driver matches the driver field */
> +#define USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER      0x02
> +
> +struct usbdevfs_disconnect_claim {
> +     unsigned int interface;
> +     unsigned int flags;
> +     char driver[USBDEVFS_MAXDRIVERNAME + 1];
> +};

You export this to user space. Please, please use u32 and u8.

        Regards
                Oliver


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Reply via email to