| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 26 Nov 2015 16:59:25 +0800
From: Peter Chen <peter.chen@...escale.com>
To: Emilio López <emilio.lopez@...labora.co.uk>
CC: <gregkh@...uxfoundation.org>, <stern@...land.harvard.edu>,
<kborer@...il.com>, <reillyg@...omium.org>,
<keescook@...omium.org>, <linux-api@...r.kernel.org>,
<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<jorgelo@...omium.org>, <dan.carpenter@...cle.com>
Subject: Re: [PATCH v1] usb: devio: Add ioctl to disallow detaching kernel
USB drivers.
On Wed, Nov 25, 2015 at 12:45:34PM -0300, Emilio López wrote:
> From: Reilly Grant <reillyg@...omium.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 <reillyg@...omium.org>
> Reviewed-by: Jorge Lucangeli Obes <jorgelo@...omium.org>
> Reviewed-by: Kees Cook <keescook@...omium.org>
> [emilio.lopez: fix build for v4.4-rc2 and adjust patch title prefix]
> Signed-off-by: Emilio López <emilio.lopez@...labora.co.uk>
>
> ---
>
> drivers/usb/core/devio.c | 50 +++++++++++++++++++++++++++++++++++----
> include/uapi/linux/usbdevice_fs.h | 1 +
> 2 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 38ae877c..a5ccc50 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -77,6 +77,7 @@ struct usb_dev_state {
> unsigned long ifclaimed;
> u32 secid;
> u32 disabled_bulk_eps;
> + bool privileges_dropped;
> };
>
> struct async {
> @@ -878,7 +879,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 +912,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;
The above changes seem to be not related with this change.
> security_task_getsecid(current, &ps->secid);
> smp_wmb();
> list_add_tail(&ps->list, &dev->filelist);
> @@ -1215,6 +1213,35 @@ static int proc_connectinfo(struct usb_dev_state *ps, void __user *arg)
>
> static int proc_resetdevice(struct usb_dev_state *ps)
> {
> + if (ps->privileges_dropped) {
> + struct usb_host_config *actconfig = ps->dev->actconfig;
> +
> + /* 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 (actconfig) {
> + int i;
> +
> + for (i = 0; i < actconfig->desc.bNumInterfaces; ++i) {
> + if (usb_interface_claimed(
> + actconfig->interface[i])) {
> + dev_warn(&ps->dev->dev,
> + "usbfs: interface %d claimed by"
> + " %s while '%s'"
> + " resets device\n",
> + actconfig->interface[i]
> + ->cur_altsetting
> + ->desc.bInterfaceNumber,
The length of line is 80 characters
> + actconfig->interface[i]
> + ->dev.driver->name,
> + current->comm);
> + return -EACCES;
> + }
> + }
> + }
> + }
> +
> return usb_reset_device(ps->dev);
> }
>
> @@ -1922,6 +1949,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) {
> @@ -2084,6 +2114,9 @@ static int proc_disconnect_claim(struct usb_dev_state *ps, void __user *arg)
> sizeof(dc.driver)) == 0)
> return -EBUSY;
>
> + if (ps->privileges_dropped)
> + return -EACCES;
> +
> dev_dbg(&intf->dev, "disconnect by usbfs\n");
> usb_driver_release_interface(driver, intf);
> }
> @@ -2130,6 +2163,12 @@ 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)
> +{
> + 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 +2357,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);
> + break;
> }
>
> done:
> diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h
> index 019ba1e..99c296b 100644
> --- a/include/uapi/linux/usbdevice_fs.h
> +++ b/include/uapi/linux/usbdevice_fs.h
> @@ -187,5 +187,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 _IO('U', 30)
>
> #endif /* _UAPI_LINUX_USBDEVICE_FS_H */
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists