From: Reilly Grant <reil...@chromium.org>

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 <reil...@chromium.org>
Signed-off-by: Emilio López <emilio.lo...@collabora.co.uk>

---

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

Reply via email to