[PATCH v2] usb: devio: Add ioctl to disallow detaching kernel USB drivers.

From: Emilio LÃpez
Date: Thu Jan 21 2016 - 18:55:20 EST

From: Reilly Grant <reillyg@xxxxxxxxxxxx>

The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily
relinquish the ability to issue other ioctls that may interfere with
other processes and drivers that have claimed an interface on the
device.

Signed-off-by: Reilly Grant <reillyg@xxxxxxxxxxxx>
Signed-off-by: Emilio LÃpez <emilio.lopez@xxxxxxxxxxxxxxx>

---

I guess code talks more than words :) This patch is only build-tested,
and is meant to showcase the approach. The basic idea is to allow
limiting the interface claims via an extra parameter to resolve
Krzysztof's worries.

A longer paragraph explaining the idea can be seen at

http://www.spinics.net/lists/linux-usb/msg135271.html

Cheers!
Emilio

Changes in v2:
- Added a parameter to the ioctl, a mask of interfaces an unprivileged
process is allowed to claim.
- Drop Tested-by's

drivers/usb/core/devio.c | 65 ++++++++++++++++++++++++++++++++++++---
include/uapi/linux/usbdevice_fs.h | 5 +++
2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..bf40aa6 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -77,6 +77,8 @@ struct usb_dev_state {
unsigned long ifclaimed;
u32 secid;
u32 disabled_bulk_eps;
+ bool privileges_dropped;
+ unsigned long interface_allowed_mask;
};

struct async {
@@ -641,6 +643,14 @@ static int claimintf(struct usb_dev_state *ps, unsigned int ifnum)
if (test_bit(ifnum, &ps->ifclaimed))
return 0;

+ if (ps->privileges_dropped) {
+ if (ifnum >= 8*sizeof(ps->interface_allowed_mask))
+ return -EINVAL;
+
+ if (!test_bit(ifnum, &ps->interface_allowed_mask))
+ return -EACCES;
+ }
+
intf = usb_ifnum_to_if(dev, ifnum);
if (!intf)
err = -ENOENT;
@@ -878,7 +888,7 @@ static int usbdev_open(struct inode *inode, struct file *file)
int ret;

ret = -ENOMEM;
- ps = kmalloc(sizeof(struct usb_dev_state), GFP_KERNEL);
+ ps = kzalloc(sizeof(struct usb_dev_state), GFP_KERNEL);
if (!ps)
goto out_free_ps;

@@ -911,11 +921,8 @@ static int usbdev_open(struct inode *inode, struct file *file)
INIT_LIST_HEAD(&ps->async_pending);
INIT_LIST_HEAD(&ps->async_completed);
init_waitqueue_head(&ps->wait);
- ps->discsignr = 0;
ps->disc_pid = get_pid(task_pid(current));
ps->cred = get_current_cred();
- ps->disccontext = NULL;
- ps->ifclaimed = 0;
security_task_getsecid(current, &ps->secid);
smp_wmb();
list_add_tail(&ps->list, &dev->filelist);
@@ -1215,6 +1222,27 @@ static int proc_connectinfo(struct usb_dev_state *ps, void __user *arg)

static int proc_resetdevice(struct usb_dev_state *ps)
{
+ struct usb_host_config *actconfig = ps->dev->actconfig;
+ struct usb_interface *interface;
+ int i, number;
+
+ /* Don't touch the device if any interfaces are claimed. It
+ * could interfere with other drivers' operations and this
+ * process has dropped its privileges to do such things.
+ */
+ if (ps->privileges_dropped && actconfig) {
+ for (i = 0; i < actconfig->desc.bNumInterfaces; ++i) {
+ interface = actconfig->interface[i];
+ number = interface->cur_altsetting->desc.bInterfaceNumber;
+ if (usb_interface_claimed(interface)) {
+ dev_warn(&ps->dev->dev,
+ "usbfs: interface %d claimed by %s while '%s' resets device\n",
+ number, interface->dev.driver->name, current->comm);
+ return -EACCES;
+ }
+ }
+ }
+
return usb_reset_device(ps->dev);
}

@@ -1922,6 +1950,9 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
struct usb_interface *intf = NULL;
struct usb_driver *driver = NULL;

+ if (ps->privileges_dropped)
+ return -EACCES;
+
/* alloc buffer */
size = _IOC_SIZE(ctl->ioctl_code);
if (size > 0) {
@@ -2074,6 +2105,9 @@ static int proc_disconnect_claim(struct usb_dev_state *ps, void __user *arg)
if (intf->dev.driver) {
struct usb_driver *driver = to_usb_driver(intf->dev.driver);

+ if (ps->privileges_dropped)
+ return -EACCES;
+
if ((dc.flags & USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER) &&
strncmp(dc.driver, intf->dev.driver->name,
sizeof(dc.driver)) != 0)
@@ -2130,6 +2164,26 @@ static int proc_free_streams(struct usb_dev_state *ps, void __user *arg)
return r;
}

+static int proc_drop_privileges(struct usb_dev_state *ps, void __user *arg)
+{
+ struct usbdevfs_drop_privs data;
+
+ if (copy_from_user(&data, arg, sizeof(data)))
+ return -EFAULT;
+
+ /* This is a one way operation. Once privileges were dropped,
+ * you cannot do it again (Otherwise unprivileged processes
+ * would be able to change their allowed interfaces mask)
+ */
+ if (ps->privileges_dropped)
+ return -EACCES;
+
+ ps->interface_allowed_mask = data.interface_allowed_mask;
+ ps->privileges_dropped = true;
+
+ return 0;
+}
+
/*
* NOTE: All requests here that have interface numbers as parameters
* are assuming that somehow the configuration has been prevented from
@@ -2318,6 +2372,9 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
case USBDEVFS_FREE_STREAMS:
ret = proc_free_streams(ps, p);
break;
+ case USBDEVFS_DROP_PRIVILEGES:
+ ret = proc_drop_privileges(ps, p);
+ break;
}

done:
diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h
index 019ba1e..9abcb34 100644
--- a/include/uapi/linux/usbdevice_fs.h
+++ b/include/uapi/linux/usbdevice_fs.h
@@ -154,6 +154,10 @@ struct usbdevfs_streams {
unsigned char eps[0];
};

+struct usbdevfs_drop_privs {
+ unsigned long interface_allowed_mask;
+};
+
#define USBDEVFS_CONTROL _IOWR('U', 0, struct usbdevfs_ctrltransfer)
#define USBDEVFS_CONTROL32 _IOWR('U', 0, struct usbdevfs_ctrltransfer32)
#define USBDEVFS_BULK _IOWR('U', 2, struct usbdevfs_bulktransfer)
@@ -187,5 +191,6 @@ struct usbdevfs_streams {
#define USBDEVFS_DISCONNECT_CLAIM _IOR('U', 27, struct usbdevfs_disconnect_claim)
#define USBDEVFS_ALLOC_STREAMS _IOR('U', 28, struct usbdevfs_streams)
#define USBDEVFS_FREE_STREAMS _IOR('U', 29, struct usbdevfs_streams)
+#define USBDEVFS_DROP_PRIVILEGES _IOR('U', 30, struct usbdevfs_drop_privs)

#endif /* _UAPI_LINUX_USBDEVICE_FS_H */
--
2.5.0