Bastien Nocera <hadess@xxxxxxxxxx> writes:
> There is a need for userspace applications to open USB devices directly,
> for all the USB devices without a kernel-level class driver[1], and
> implemented in user-space.
>
> As not all devices are built equal, we want to be able to revoke
> access to those devices whether it's because the user isn't at the
> console anymore, or because the web browser, or sandbox doesn't want
> to allow access to that device.
>
> This commit implements the internal API used to revoke access to USB
> devices, given either bus and device numbers, or/and a user's
> effective UID.
>
> Signed-off-by: Bastien Nocera <hadess@xxxxxxxxxx>
>
> [1]:
> Non exhaustive list of devices and device types that need raw USB access:
> - all manners of single-board computers and programmable chips and
> devices (avrdude, STLink, sunxi bootloader, flashrom, etc.)
> - 3D printers
> - scanners
> - LCD "displays"
> - user-space webcam and still cameras
> - game controllers
> - video/audio capture devices
> - sensors
> - software-defined radios
> - DJ/music equipment
> - protocol analysers
> - Rio 500 music player
> ---
> drivers/usb/core/devio.c | 79 ++++++++++++++++++++++++++++++++++++++--
> drivers/usb/core/usb.h | 2 +
> 2 files changed, 77 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index b5b85bf80329..d8d212ef581f 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -78,6 +78,7 @@ struct usb_dev_state {
> int not_yet_resumed;
> bool suspend_allowed;
> bool privileges_dropped;
> + bool revoked;
> };
>
> struct usb_memory {
> @@ -183,6 +184,13 @@ static int connected(struct usb_dev_state *ps)
> ps->dev->state != USB_STATE_NOTATTACHED);
> }
>
> +static int disconnected_or_revoked(struct usb_dev_state *ps)
> +{
> + return (list_empty(&ps->list) ||
> + ps->dev->state == USB_STATE_NOTATTACHED ||
> + ps->revoked);
> +}
> +
> static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
> {
> struct usb_dev_state *ps = usbm->ps;
> @@ -237,6 +245,9 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
> dma_addr_t dma_handle;
> int ret;
>
> + if (disconnected_or_revoked(ps))
> + return -ENODEV;
> +
> ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
> if (ret)
> goto error;
> @@ -310,7 +321,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes,
>
> pos = *ppos;
> usb_lock_device(dev);
> - if (!connected(ps)) {
> + if (disconnected_or_revoked(ps)) {
> ret = -ENODEV;
> goto err;
> } else if (pos < 0) {
> @@ -2315,7 +2326,7 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
> if (ps->privileges_dropped)
> return -EACCES;
>
> - if (!connected(ps))
> + if (disconnected_or_revoked(ps))
> return -ENODEV;
>
> /* alloc buffer */
> @@ -2555,7 +2566,7 @@ static int proc_forbid_suspend(struct usb_dev_state *ps)
>
> static int proc_allow_suspend(struct usb_dev_state *ps)
> {
> - if (!connected(ps))
> + if (disconnected_or_revoked(ps))
> return -ENODEV;
>
> WRITE_ONCE(ps->not_yet_resumed, 1);
> @@ -2580,6 +2591,66 @@ static int proc_wait_for_resume(struct usb_dev_state *ps)
> return proc_forbid_suspend(ps);
> }
>
> +static int usbdev_revoke(struct usb_dev_state *ps)
> +{
> + struct usb_device *dev = ps->dev;
> + unsigned int ifnum;
> + struct async *as;
> +
> + if (ps->revoked) {
> + usb_unlock_device(dev);
> + return -ENODEV;
> + }
> +
> + snoop(&dev->dev, "%s: REVOKE by PID %d: %s\n", __func__,
> + task_pid_nr(current), current->comm);
> +
> + ps->revoked = true;
> +
> + for (ifnum = 0; ps->ifclaimed && ifnum < 8*sizeof(ps->ifclaimed);
> + ifnum++) {
> + if (test_bit(ifnum, &ps->ifclaimed))
> + releaseintf(ps, ifnum);
> + }
> + destroy_all_async(ps);
> +
> + as = async_getcompleted(ps);
> + while (as) {
> + free_async(as);
> + as = async_getcompleted(ps);
> + }
> +
> + wake_up(&ps->wait);
> +
> + return 0;
> +}
> +
> +int usb_revoke_for_euid(struct usb_device *udev,
> + int euid)
^^^
At a minimum that should be a kuid_t.
> +{
> + struct usb_dev_state *ps;
> +
> + usb_lock_device(udev);
> +
> + list_for_each_entry(ps, &udev->filelist, list) {
> + if (euid >= 0) {
^^^^^^^^^
That test should be uid_valid.
> + kuid_t kuid;
> +
> + if (!ps || !ps->cred)
> + continue;
> + kuid = ps->cred->euid;
> + if (kuid.val != euid)
^^^^^^^^^^^^^^^^^^^^^
That test should be if (!uid_eq(ps->cred->euid, euid))
The point is that inside the kernel all uid data should be dealt with
in the kuid_t data type. So as to avoid confusing uids with some other
kind of integer data.
> + continue;
> + }
> +
> + usbdev_revoke(ps);
> + }
> +
> +out:
> + usb_unlock_device(udev);
> + return 0;
> +}
> +
> /*
> * NOTE: All requests here that have interface numbers as parameters
> * are assuming that somehow the configuration has been prevented from
> @@ -2623,7 +2694,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
> #endif
> }
>
> - if (!connected(ps)) {
> + if (disconnected_or_revoked(ps)) {
> usb_unlock_device(dev);
> return -ENODEV;
> }
> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> index 82538daac8b8..5b007530a1cf 100644
> --- a/drivers/usb/core/usb.h
> +++ b/drivers/usb/core/usb.h
> @@ -34,6 +34,8 @@ extern int usb_deauthorize_device(struct usb_device *);
> extern int usb_authorize_device(struct usb_device *);
> extern void usb_deauthorize_interface(struct usb_interface *);
> extern void usb_authorize_interface(struct usb_interface *);
> +extern int usb_revoke_for_euid(struct usb_device *udev,
> + int euid);
> extern void usb_detect_quirks(struct usb_device *udev);
> extern void usb_detect_interface_quirks(struct usb_device *udev);
> extern void usb_release_quirk_list(void);
Eric
