Re: [PATCH] usbfs: Add a new disconnect-and-claim ioctl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <hdegoede@xxxxxxxxxx>
> ---
>  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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux