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