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

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

 



Hi Alan,

El 22/01/16 a las 13:10, Alan Stern escribió:
On Thu, 21 Jan 2016, Emilio López wrote:

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>


  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.
+	 */

This comment should be rephrased.  It should say something like:
"Don't allow if the process has dropped its privilege to do such
things and any of the interfaces are claimed."

I have replaced it with the following now

        /* Don't allow a device reset if the process has dropped the
         * privilege to do such things and any of the interfaces are
         * currently claimed.
         */

You also might consider allowing the reset if the interfaces are
claimed only by the current process (or more precisely, by ps).

+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 you're going to keep a mask of claimable interfaces then there's no
reason this has to be a one-time operation.  Processes should always be
allowed to shrink the mask, just not to grow it.

Good point, I've changed this to look like the following

        /* This is an one way operation. Once privileges are
         * dropped, you cannot regain them. You may however reissue
         * this ioctl to shrink the allowed interfaces mask.
         */
        if (ps->privileges_dropped)
                ps->interface_allowed_mask &= data.interface_allowed_mask;
        else
                ps->interface_allowed_mask = data.interface_allowed_mask;

        ps->privileges_dropped = true;

Or maybe I could change the default mask to ~0 and simplify this a bit, hm.

Thank you for the review!

Emilio
--
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