On Thu, Nov 26, 2015 at 04:59:25PM +0800, Peter Chen wrote:
> On Wed, Nov 25, 2015 at 12:45:34PM -0300, 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>
> > Reviewed-by: Jorge Lucangeli Obes <jorg...@chromium.org>
> > Reviewed-by: Kees Cook <keesc...@chromium.org>
> > [emilio.lopez: fix build for v4.4-rc2 and adjust patch title prefix]
> > Signed-off-by: Emilio López <emilio.lo...@collabora.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.
>
It is though. They added a new struct member and thought that now it
was cleaner to use kzalloc() instead of initializing the new member to
zero.
> > 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;
> > + }
> > + }
> > + }
> > + }
Yeah. Better to flip this around:
static int proc_resetdevice(struct usb_dev_state *ps)
{
struct usb_host_config *actconfig = ps->dev->actconfig;
struct usb_interface *interface;
int i;
if (ps->privileges_dropped && actconfig) {
for (i = 0; i < actconfig->desc.bNumInterfaces; ++i) {
interface = actconfig->interface[i];
if (usb_interface_claimed(interface)) {
dev_warn(&ps->dev->dev,
"usbfs: interface %d claimed by %s
while '%s' resets device\n",
interface->cur_altsetting->desc.bInterfaceNumber,
interface->dev.driver->name,
current->comm);
return -EACCES;
}
}
}
return usb_reset_device(ps->dev);
}
It still goes over the 80 character limit, but that's cleaner than
breaking up the variable.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
