Re: [PATCH v8 2/7] usb: interface authorization: Introduces the default interface authorization

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

 



On Tue, Aug 04, 2015 at 10:04:13PM +0200, Stefan Koch wrote:
> Am Dienstag, den 04.08.2015, 12:14 -0700 schrieb Greg KH:
> > On Tue, Aug 04, 2015 at 08:55:38PM +0200, Stefan Koch wrote:
> > > Am Montag, den 03.08.2015, 15:06 -0700 schrieb Greg KH:
> > > > On Thu, Jul 30, 2015 at 08:19:19AM +0200, Stefan Koch wrote:
> > > > > Interfaces are allowed per default.
> > > > > This can disabled or enabled (again) by writing 0 or 1 to
> > > > > /sys/bus/usb/devices/usbX/interface_authorized_default
> > > > > 
> > > > > Signed-off-by: Stefan Koch <skoch@xxxxxxx>
> > > > > ---
> > > > >  drivers/usb/core/hcd.c     | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  drivers/usb/core/message.c |  1 +
> > > > >  include/linux/usb/hcd.h    |  3 +++
> > > > >  3 files changed, 51 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > > > > index cbcd092..84b5923 100644
> > > > > --- a/drivers/usb/core/hcd.c
> > > > > +++ b/drivers/usb/core/hcd.c
> > > > > @@ -882,9 +882,53 @@ static ssize_t authorized_default_store(struct device *dev,
> > > > >  }
> > > > >  static DEVICE_ATTR_RW(authorized_default);
> > > > >  
> > > > > +/*
> > > > > + * interface_authorized_default_show - show default authorization status
> > > > > + * for USB interfaces
> > > > > + *
> > > > > + * note: interface_authorized_default is the default value
> > > > > + *       for initializing the authorized attribute of interfaces
> > > > > + */
> > > > > +static ssize_t interface_authorized_default_show(struct device *dev,
> > > > > +		struct device_attribute *attr, char *buf)
> > > > > +{
> > > > > +	struct usb_device *usb_dev = to_usb_device(dev);
> > > > > +	struct usb_hcd *hcd = bus_to_hcd(usb_dev->bus);
> > > > > +
> > > > > +	return sprintf(buf, "%u\n", !!HCD_INTF_AUTHORIZED(hcd));
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * interface_authorized_default_store - store default authorization status
> > > > > + * for USB interfaces
> > > > > + *
> > > > > + * note: interface_authorized_default is the default value
> > > > > + *       for initializing the authorized attribute of interfaces
> > > > > + */
> > > > > +static ssize_t interface_authorized_default_store(struct device *dev,
> > > > > +		struct device_attribute *attr, const char *buf, size_t count)
> > > > > +{
> > > > > +	struct usb_device *usb_dev = to_usb_device(dev);
> > > > > +	struct usb_hcd *hcd = bus_to_hcd(usb_dev->bus);
> > > > > +	int rc = count;
> > > > > +	bool val;
> > > > > +
> > > > > +	if (strtobool(buf, &val) != 0)
> > > > > +		return -EINVAL;
> > > > > +
> > > > > +	if (val)
> > > > > +		set_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
> > > > > +	else
> > > > > +		clear_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
> > > > > +
> > > > > +	return rc;
> > > > > +}
> > > > > +static DEVICE_ATTR_RW(interface_authorized_default);
> > > > > +
> > > > >  /* Group all the USB bus attributes */
> > > > >  static struct attribute *usb_bus_attrs[] = {
> > > > >  		&dev_attr_authorized_default.attr,
> > > > > +		&dev_attr_interface_authorized_default.attr,
> > > > >  		NULL,
> > > > >  };
> > > > >  
> > > > > @@ -2682,6 +2726,9 @@ int usb_add_hcd(struct usb_hcd *hcd,
> > > > >  		hcd->authorized_default = authorized_default;
> > > > >  	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> > > > >  
> > > > > +	/* per default all interfaces are authorized */
> > > > > +	set_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
> > > > > +
> > > > >  	/* HC is in reset state, but accessible.  Now do the one-time init,
> > > > >  	 * bottom up so that hcds can customize the root hubs before hub_wq
> > > > >  	 * starts talking to them.  (Note, bus id is assigned early too.)
> > > > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> > > > > index f368d20..3d25d89 100644
> > > > > --- a/drivers/usb/core/message.c
> > > > > +++ b/drivers/usb/core/message.c
> > > > > @@ -1807,6 +1807,7 @@ free_interfaces:
> > > > >  		intfc = cp->intf_cache[i];
> > > > >  		intf->altsetting = intfc->altsetting;
> > > > >  		intf->num_altsetting = intfc->num_altsetting;
> > > > > +		intf->authorized = !!HCD_INTF_AUTHORIZED(hcd);
> > > > 
> > > > 
> > > > 
> > > > >  		kref_get(&intfc->ref);
> > > > >  
> > > > >  		alt = usb_altnum_to_altsetting(intf, 0);
> > > > > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> > > > > index c9aa779..7b8ddae 100644
> > > > > --- a/include/linux/usb/hcd.h
> > > > > +++ b/include/linux/usb/hcd.h
> > > > > @@ -120,6 +120,7 @@ struct usb_hcd {
> > > > >  #define HCD_FLAG_WAKEUP_PENDING		4	/* root hub is resuming? */
> > > > >  #define HCD_FLAG_RH_RUNNING		5	/* root hub is running? */
> > > > >  #define HCD_FLAG_DEAD			6	/* controller has died? */
> > > > > +#define HCD_FLAG_INTF_AUTHORIZED	8	/* authorize interfaces? */
> > > > 
> > > > Again, what happened to this being '7'?  Don't skip bits for no reason.
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > Because this reason patch 7 was developed. It changes the device
> > > authorization to use a flag with the desired number 6.
> > 
> > "6"?
> Sorry desired was "7". Parch 7 introduces flag nr. "7"
> 
> +#define HCD_FLAG_DEV_AUTHORIZED                7       /* authorize
> devices? */
>  #define HCD_FLAG_INTF_AUTHORIZED       8       /* authorize interfaces?
> */
> 
> > 
> > > Is this ok or not?
> > 
> > In reading these patches in the order you have sent them, no, it's not,
> > I'm totally confused,
> 
> The reason for choosing 7 for dev auth was that the device authorization
> was there before the interface autorization (it is the "parent").

That doesn't really matter.

> So what do you prefer?
> a) 8 for dev auth, 7 for intf auth and submit the dev auth change as
> last patch
> b) 7 for dev auth, 8 for intf auth (as is) and submit the dev auth
> change as first patch
> c) 7 for dev auth, 8 for intf auth and submit the dev auth change as
> first patch

Don't leave holes in the series when you submit the patches, the numbers
don't matter to me as to which is which.

thanks,

greg k-h
--
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