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 <[email protected]>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
