| Message ID | 20210930010511.3387967-2-sathyanarayanan.kuppuswamy@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Add device filter support | expand |
On Wed, Sep 29, 2021 at 06:05:06PM -0700, Kuppuswamy Sathyanarayanan wrote: > Currently bus drivers like "USB" or "Thunderbolt" implement a custom > version of device authorization to selectively authorize the driver > probes. Since there is a common requirement, move the "authorized" > attribute support to the driver core in order to allow it to be used > by other subsystems / buses. > > Similar requirements have been discussed in the PCI [1] community for > PCI bus drivers as well. > > No functional changes are intended. It just converts authorized > attribute from int to bool and moves it to the driver core. There > should be no user-visible change in the location or semantics of > attributes for USB devices. > > Regarding thunderbolt driver, although it declares sw->authorized as > "int" and allows 0,1,2 as valid values for sw->authorized attribute, > but within the driver, in all authorized attribute related checks, > it is treated as bool value. So when converting the authorized > attribute from int to bool value, there should be no functional > changes other than value 2 being not visible to the user. > > [1]: https://lore.kernel.org/all/CACK8Z6E8pjVeC934oFgr=VB3pULx_GyT2NkzAogdRQJ9TKSX9A@mail.gmail.com/ > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> Since you're moving the authorized flag from the USB core to the driver core, the corresponding sysfs attribute functions should be moved as well. Also, you ignored the usb_[de]authorize_interface() functions and their friends. Alan Stern
On Wed, Sep 29, 2021 at 6:43 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Wed, Sep 29, 2021 at 06:05:06PM -0700, Kuppuswamy Sathyanarayanan wrote: > > Currently bus drivers like "USB" or "Thunderbolt" implement a custom > > version of device authorization to selectively authorize the driver > > probes. Since there is a common requirement, move the "authorized" > > attribute support to the driver core in order to allow it to be used > > by other subsystems / buses. > > > > Similar requirements have been discussed in the PCI [1] community for > > PCI bus drivers as well. > > > > No functional changes are intended. It just converts authorized > > attribute from int to bool and moves it to the driver core. There > > should be no user-visible change in the location or semantics of > > attributes for USB devices. > > > > Regarding thunderbolt driver, although it declares sw->authorized as > > "int" and allows 0,1,2 as valid values for sw->authorized attribute, > > but within the driver, in all authorized attribute related checks, > > it is treated as bool value. So when converting the authorized > > attribute from int to bool value, there should be no functional > > changes other than value 2 being not visible to the user. > > > > [1]: https://lore.kernel.org/all/CACK8Z6E8pjVeC934oFgr=VB3pULx_GyT2NkzAogdRQJ9TKSX9A@mail.gmail.com/ > > > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > Since you're moving the authorized flag from the USB core to the > driver core, the corresponding sysfs attribute functions should be > moved as well. Unlike when 'removable' moved from USB to the driver core there isn't a common definition for how the 'authorized' sysfs-attribute behaves across buses. The only common piece is where this flag is stored in the data structure, i.e. the 'authorized' sysfs interface is purposefully left bus specific. > Also, you ignored the usb_[de]authorize_interface() functions and > their friends. Ugh, yes.
On 9/29/21 6:55 PM, Dan Williams wrote: >> Also, you ignored the usb_[de]authorize_interface() functions and >> their friends. > Ugh, yes. I did not change it because I am not sure about the interface vs device dependency. I think following change should work. diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index f57b5a7a90ca..84969732d09c 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -334,7 +334,7 @@ static int usb_probe_interface(struct device *dev) if (udev->dev.authorized == false) { dev_err(&intf->dev, "Device is not authorized for usage\n"); return error; - } else if (intf->authorized == 0) { + } else if (intf->dev.authorized == 0) { dev_err(&intf->dev, "Interface %d is not authorized for usage\n", intf->altsetting->desc.bInterfaceNumber); return error; @@ -546,7 +546,7 @@ int usb_driver_claim_interface(struct usb_driver *driver, return -EBUSY; /* reject claim if interface is not authorized */ - if (!iface->authorized) + if (!iface->dev.authorized) return -ENODEV; dev->driver = &driver->drvwrap.driver; diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 47548ce1cfb1..ab3c8d1e4db9 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1791,9 +1791,9 @@ void usb_deauthorize_interface(struct usb_interface *intf) device_lock(dev->parent); - if (intf->authorized) { + if (intf->dev.authorized) { device_lock(dev); - intf->authorized = 0; + intf->dev.authorized = 0; device_unlock(dev); usb_forced_unbind_intf(intf); @@ -1811,9 +1811,9 @@ void usb_authorize_interface(struct usb_interface *intf) { struct device *dev = &intf->dev; - if (!intf->authorized) { + if (!intf->dev.authorized) { device_lock(dev); - intf->authorized = 1; /* authorize interface */ + intf->dev.authorized = 1; /* authorize interface */ device_unlock(dev); } } @@ -2069,7 +2069,6 @@ int usb_set_configuration(struct usb_device *dev, int configuration) 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); @@ -2101,6 +2100,7 @@ int usb_set_configuration(struct usb_device *dev, int configuration) INIT_WORK(&intf->reset_ws, __usb_queue_reset_device); intf->minor = -1; device_initialize(&intf->dev); + intf->dev.authorized = !!HCD_INTF_AUTHORIZED(hcd); pm_runtime_no_callbacks(&intf->dev); dev_set_name(&intf->dev, "%d-%s:%d.%d", dev->bus->busnum, dev->devpath, configuration, ifnum); diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 3d63e345d0a0..666eeb80ff29 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -1160,9 +1160,7 @@ static DEVICE_ATTR_RO(supports_autosuspend); static ssize_t interface_authorized_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct usb_interface *intf = to_usb_interface(dev); - - return sprintf(buf, "%u\n", intf->authorized); + return sprintf(buf, "%u\n", dev->authorized); } /* diff --git a/include/linux/usb.h b/include/linux/usb.h index 796df4068e94..1e41453c63cb 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -195,8 +195,6 @@ usb_find_last_int_out_endpoint(struct usb_host_interface *alt, * has been deferred. * @needs_binding: flag set when the driver should be re-probed or unbound * following a reset or suspend operation it doesn't support. - * @authorized: This allows to (de)authorize individual interfaces instead - * a whole device in contrast to the device authorization. * @dev: driver model's view of this device * @usb_dev: if an interface is bound to the USB major, this will point * to the sysfs representation for that device. @@ -252,7 +250,6 @@ struct usb_interface { unsigned needs_altsetting0:1; /* switch to altsetting 0 is pending */ unsigned needs_binding:1; /* needs delayed unbind/rebind */ unsigned resetting_device:1; /* true: bandwidth alloc after reset */ - unsigned authorized:1; /* used for interface authorization */ struct device dev; /* interface specific device info */ struct device *usb_dev;
On Wed, Sep 29, 2021 at 7:39 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > On 9/29/21 6:55 PM, Dan Williams wrote: > >> Also, you ignored the usb_[de]authorize_interface() functions and > >> their friends. > > Ugh, yes. > > I did not change it because I am not sure about the interface vs device > dependency. > This is was the rationale for has_probe_authorization flag. USB performs authorization of child devices based on the authorization state of the parent interface. > I think following change should work. > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > index f57b5a7a90ca..84969732d09c 100644 > --- a/drivers/usb/core/driver.c > +++ b/drivers/usb/core/driver.c > @@ -334,7 +334,7 @@ static int usb_probe_interface(struct device *dev) > if (udev->dev.authorized == false) { > dev_err(&intf->dev, "Device is not authorized for usage\n"); > return error; > - } else if (intf->authorized == 0) { > + } else if (intf->dev.authorized == 0) { == false. > dev_err(&intf->dev, "Interface %d is not authorized for usage\n", > intf->altsetting->desc.bInterfaceNumber); > return error; > @@ -546,7 +546,7 @@ int usb_driver_claim_interface(struct usb_driver *driver, > return -EBUSY; > > /* reject claim if interface is not authorized */ > - if (!iface->authorized) > + if (!iface->dev.authorized) I'd do == false to keep it consistent with other conversions. > return -ENODEV; > > dev->driver = &driver->drvwrap.driver; > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > index 47548ce1cfb1..ab3c8d1e4db9 100644 > --- a/drivers/usb/core/message.c > +++ b/drivers/usb/core/message.c > @@ -1791,9 +1791,9 @@ void usb_deauthorize_interface(struct usb_interface *intf) > > device_lock(dev->parent); > > - if (intf->authorized) { > + if (intf->dev.authorized) { > device_lock(dev); > - intf->authorized = 0; > + intf->dev.authorized = 0; = false; > device_unlock(dev); > > usb_forced_unbind_intf(intf); > @@ -1811,9 +1811,9 @@ void usb_authorize_interface(struct usb_interface *intf) > { > struct device *dev = &intf->dev; > > - if (!intf->authorized) { > + if (!intf->dev.authorized) { > device_lock(dev); > - intf->authorized = 1; /* authorize interface */ > + intf->dev.authorized = 1; /* authorize interface */ = true ...not sure that comment is worth preserving.
On Thu, Sep 30, 2021 at 6:59 AM Dan Williams <dan.j.williams@intel.com> wrote: > > On Wed, Sep 29, 2021 at 7:39 PM Kuppuswamy, Sathyanarayanan > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > > > > > On 9/29/21 6:55 PM, Dan Williams wrote: > > >> Also, you ignored the usb_[de]authorize_interface() functions and > > >> their friends. > > > Ugh, yes. > > > > I did not change it because I am not sure about the interface vs device > > dependency. > > > > This is was the rationale for has_probe_authorization flag. USB > performs authorization of child devices based on the authorization > state of the parent interface. > > > I think following change should work. > > > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > > index f57b5a7a90ca..84969732d09c 100644 > > --- a/drivers/usb/core/driver.c > > +++ b/drivers/usb/core/driver.c > > @@ -334,7 +334,7 @@ static int usb_probe_interface(struct device *dev) > > if (udev->dev.authorized == false) { > > dev_err(&intf->dev, "Device is not authorized for usage\n"); > > return error; > > - } else if (intf->authorized == 0) { > > + } else if (intf->dev.authorized == 0) { > > == false Or even (!intf->dev.authorized). > > dev_err(&intf->dev, "Interface %d is not authorized for usage\n", > > intf->altsetting->desc.bInterfaceNumber); > > return error; > > @@ -546,7 +546,7 @@ int usb_driver_claim_interface(struct usb_driver *driver, > > return -EBUSY; > > > > /* reject claim if interface is not authorized */ > > - if (!iface->authorized) > > + if (!iface->dev.authorized) > > I'd do == false to keep it consistent with other conversions. But this looks odd, FWIW.
On Thu, Sep 30, 2021 at 4:05 AM Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > no functional > changes other than value 2 being not visible to the user. > Are we sure we don't break any user-facing tool with it? Tools might use this to "remember" how the device was authorized this time.
On Wed, Sep 29, 2021 at 06:55:12PM -0700, Dan Williams wrote: > On Wed, Sep 29, 2021 at 6:43 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > On Wed, Sep 29, 2021 at 06:05:06PM -0700, Kuppuswamy Sathyanarayanan wrote: > > > Currently bus drivers like "USB" or "Thunderbolt" implement a custom > > > version of device authorization to selectively authorize the driver > > > probes. Since there is a common requirement, move the "authorized" > > > attribute support to the driver core in order to allow it to be used > > > by other subsystems / buses. > > > > > > Similar requirements have been discussed in the PCI [1] community for > > > PCI bus drivers as well. > > > > > > No functional changes are intended. It just converts authorized > > > attribute from int to bool and moves it to the driver core. There > > > should be no user-visible change in the location or semantics of > > > attributes for USB devices. > > > > > > Regarding thunderbolt driver, although it declares sw->authorized as > > > "int" and allows 0,1,2 as valid values for sw->authorized attribute, > > > but within the driver, in all authorized attribute related checks, > > > it is treated as bool value. So when converting the authorized > > > attribute from int to bool value, there should be no functional > > > changes other than value 2 being not visible to the user. > > > > > > [1]: https://lore.kernel.org/all/CACK8Z6E8pjVeC934oFgr=VB3pULx_GyT2NkzAogdRQJ9TKSX9A@mail.gmail.com/ > > > > > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > > > Since you're moving the authorized flag from the USB core to the > > driver core, the corresponding sysfs attribute functions should be > > moved as well. > > Unlike when 'removable' moved from USB to the driver core there isn't > a common definition for how the 'authorized' sysfs-attribute behaves > across buses. The only common piece is where this flag is stored in > the data structure, i.e. the 'authorized' sysfs interface is > purposefully left bus specific. How about implementing "library" versions of show_authorized() and store_authorized() that the bus-specific attribute routines can call? These library routines would handle parsing the input values, storing the new flag, and displaying the stored flag value. That way at least the common parts of these APIs would be centralized in the driver core, and any additional functionality could easily be added by the bus-specific attribute routine. Alan Stern
On Thu, Sep 30, 2021 at 8:00 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Wed, Sep 29, 2021 at 06:55:12PM -0700, Dan Williams wrote: > > On Wed, Sep 29, 2021 at 6:43 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > > > On Wed, Sep 29, 2021 at 06:05:06PM -0700, Kuppuswamy Sathyanarayanan wrote: > > > > Currently bus drivers like "USB" or "Thunderbolt" implement a custom > > > > version of device authorization to selectively authorize the driver > > > > probes. Since there is a common requirement, move the "authorized" > > > > attribute support to the driver core in order to allow it to be used > > > > by other subsystems / buses. > > > > > > > > Similar requirements have been discussed in the PCI [1] community for > > > > PCI bus drivers as well. > > > > > > > > No functional changes are intended. It just converts authorized > > > > attribute from int to bool and moves it to the driver core. There > > > > should be no user-visible change in the location or semantics of > > > > attributes for USB devices. > > > > > > > > Regarding thunderbolt driver, although it declares sw->authorized as > > > > "int" and allows 0,1,2 as valid values for sw->authorized attribute, > > > > but within the driver, in all authorized attribute related checks, > > > > it is treated as bool value. So when converting the authorized > > > > attribute from int to bool value, there should be no functional > > > > changes other than value 2 being not visible to the user. > > > > > > > > [1]: https://lore.kernel.org/all/CACK8Z6E8pjVeC934oFgr=VB3pULx_GyT2NkzAogdRQJ9TKSX9A@mail.gmail.com/ > > > > > > > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > > > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > > > > > Since you're moving the authorized flag from the USB core to the > > > driver core, the corresponding sysfs attribute functions should be > > > moved as well. > > > > Unlike when 'removable' moved from USB to the driver core there isn't > > a common definition for how the 'authorized' sysfs-attribute behaves > > across buses. The only common piece is where this flag is stored in > > the data structure, i.e. the 'authorized' sysfs interface is > > purposefully left bus specific. > > How about implementing "library" versions of show_authorized() and > store_authorized() that the bus-specific attribute routines can call? > These library routines would handle parsing the input values, storing > the new flag, and displaying the stored flag value. That way at > least the common parts of these APIs would be centralized in the > driver core, and any additional functionality could easily be added > by the bus-specific attribute routine. > While show_authorized() seems like it could be standardized, have a look at what the different store_authorized() implementations do. Thunderbolt wants "switch approval" vs "switch challenge" and USB has a bunch of bus-specific work to do when the authorization state changes. I don't see much room for a library to help there as more buses add authorization support. That said I do think it would be useful to have a common implementation available for generic probe authorization to toggle the flag if the bus does not have any authorization work to do, but that seems a follow-on once this core is accepted.
On Thu, Sep 30, 2021 at 4:20 AM Yehezkel Bernat <yehezkelshb@gmail.com> wrote: > > On Thu, Sep 30, 2021 at 4:05 AM Kuppuswamy Sathyanarayanan > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > no functional > > changes other than value 2 being not visible to the user. > > > > Are we sure we don't break any user-facing tool with it? Tools might use this to > "remember" how the device was authorized this time. That's why it was highlighted in the changelog. Hopefully a Thunderbolt developer can confirm if it is a non-issue. Documentation/ABI/testing/sysfs-bus-thunderbolt does not seem to answer this question about whether authorized_show and authorized_store need to be symmetric.
On Thu, Sep 30, 2021 at 6:28 PM Dan Williams <dan.j.williams@intel.com> wrote: > > On Thu, Sep 30, 2021 at 4:20 AM Yehezkel Bernat <yehezkelshb@gmail.com> wrote: > > > > On Thu, Sep 30, 2021 at 4:05 AM Kuppuswamy Sathyanarayanan > > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > > > no functional > > > changes other than value 2 being not visible to the user. > > > > > > > Are we sure we don't break any user-facing tool with it? Tools might use this to > > "remember" how the device was authorized this time. > > That's why it was highlighted in the changelog. Hopefully a > Thunderbolt developer can confirm if it is a non-issue. > Documentation/ABI/testing/sysfs-bus-thunderbolt does not seem to > answer this question about whether authorized_show and > authorized_store need to be symmetric. Apparently, Bolt does read it [1] and cares about it [2]. [1] https://gitlab.freedesktop.org/bolt/bolt/-/blob/130e09d1c7ff02c09e4ad1c9c36e9940b68e58d8/boltd/bolt-sysfs.c#L511 [2] https://gitlab.freedesktop.org/bolt/bolt/-/blob/130e09d1c7ff02c09e4ad1c9c36e9940b68e58d8/boltd/bolt-device.c#L639
On Thu, Sep 30, 2021 at 11:25 AM Yehezkel Bernat <yehezkelshb@gmail.com> wrote: > > On Thu, Sep 30, 2021 at 6:28 PM Dan Williams <dan.j.williams@intel.com> wrote: > > > > On Thu, Sep 30, 2021 at 4:20 AM Yehezkel Bernat <yehezkelshb@gmail.com> wrote: > > > > > > On Thu, Sep 30, 2021 at 4:05 AM Kuppuswamy Sathyanarayanan > > > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > > > > > no functional > > > > changes other than value 2 being not visible to the user. > > > > > > > > > > Are we sure we don't break any user-facing tool with it? Tools might use this to > > > "remember" how the device was authorized this time. > > > > That's why it was highlighted in the changelog. Hopefully a > > Thunderbolt developer can confirm if it is a non-issue. > > Documentation/ABI/testing/sysfs-bus-thunderbolt does not seem to > > answer this question about whether authorized_show and > > authorized_store need to be symmetric. > > Apparently, Bolt does read it [1] and cares about it [2]. Ah, thank you! Yeah, looks like the conversion to bool was indeed too hopeful. > > [1] https://gitlab.freedesktop.org/bolt/bolt/-/blob/130e09d1c7ff02c09e4ad1c9c36e9940b68e58d8/boltd/bolt-sysfs.c#L511 > [2] https://gitlab.freedesktop.org/bolt/bolt/-/blob/130e09d1c7ff02c09e4ad1c9c36e9940b68e58d8/boltd/bolt-device.c#L639
On 9/30/21 12:04 PM, Dan Williams wrote: >>> That's why it was highlighted in the changelog. Hopefully a >>> Thunderbolt developer can confirm if it is a non-issue. >>> Documentation/ABI/testing/sysfs-bus-thunderbolt does not seem to >>> answer this question about whether authorized_show and >>> authorized_store need to be symmetric. >> Apparently, Bolt does read it [1] and cares about it [2]. > Ah, thank you! > > Yeah, looks like the conversion to bool was indeed too hopeful. > IIUC, the end result of value "2" in authorized sysfs is to just "authorize" or "de-authorize". In that case, can the user space driver adapt to this int->bool change? Just want to know the possibility.
On Thu, Sep 30, 2021 at 12:50 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > On 9/30/21 12:04 PM, Dan Williams wrote: > >>> That's why it was highlighted in the changelog. Hopefully a > >>> Thunderbolt developer can confirm if it is a non-issue. > >>> Documentation/ABI/testing/sysfs-bus-thunderbolt does not seem to > >>> answer this question about whether authorized_show and > >>> authorized_store need to be symmetric. > >> Apparently, Bolt does read it [1] and cares about it [2]. > > Ah, thank you! > > > > Yeah, looks like the conversion to bool was indeed too hopeful. > > > > IIUC, the end result of value "2" in authorized sysfs is to just > "authorize" or "de-authorize". In that case, can the user space > driver adapt to this int->bool change? Just want to know the > possibility. ABIs are forever. The kernel has to uphold its contract to bolt that it will return '2' and not '1' after '2' has been written.
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c index 7018d959f775..3e39686eff14 100644 --- a/drivers/thunderbolt/domain.c +++ b/drivers/thunderbolt/domain.c @@ -656,7 +656,7 @@ int tb_domain_approve_switch(struct tb *tb, struct tb_switch *sw) /* The parent switch must be authorized before this one */ parent_sw = tb_to_switch(sw->dev.parent); - if (!parent_sw || !parent_sw->authorized) + if (!parent_sw || !parent_sw->dev.authorized) return -EINVAL; return tb->cm_ops->approve_switch(tb, sw); @@ -683,7 +683,7 @@ int tb_domain_approve_switch_key(struct tb *tb, struct tb_switch *sw) /* The parent switch must be authorized before this one */ parent_sw = tb_to_switch(sw->dev.parent); - if (!parent_sw || !parent_sw->authorized) + if (!parent_sw || !parent_sw->dev.authorized) return -EINVAL; ret = tb->cm_ops->add_switch_key(tb, sw); @@ -720,7 +720,7 @@ int tb_domain_challenge_switch_key(struct tb *tb, struct tb_switch *sw) /* The parent switch must be authorized before this one */ parent_sw = tb_to_switch(sw->dev.parent); - if (!parent_sw || !parent_sw->authorized) + if (!parent_sw || !parent_sw->dev.authorized) return -EINVAL; get_random_bytes(challenge, sizeof(challenge)); diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c index 6255f1ef9599..f5b784c1cabb 100644 --- a/drivers/thunderbolt/icm.c +++ b/drivers/thunderbolt/icm.c @@ -768,7 +768,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr) * sure our book keeping matches that. */ if (sw->depth == depth && sw_phy_port == phy_port && - !!sw->authorized == authorized) { + sw->dev.authorized == authorized) { /* * It was enumerated through another link so update * route string accordingly. @@ -849,7 +849,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr) sw->connection_key = pkg->connection_key; sw->link = link; sw->depth = depth; - sw->authorized = authorized; + sw->dev.authorized = authorized; sw->security_level = security_level; sw->boot = boot; sw->link_speed = speed_gen3 ? 20 : 10; @@ -1235,7 +1235,8 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr, sw = tb_switch_find_by_uuid(tb, &pkg->ep_uuid); if (sw) { /* Update the switch if it is still in the same place */ - if (tb_route(sw) == route && !!sw->authorized == authorized) { + if (tb_route(sw) == route && + sw->dev.authorized == authorized) { parent_sw = tb_to_switch(sw->dev.parent); update_switch(parent_sw, sw, route, pkg->connection_id, 0, 0, 0, boot); @@ -1272,7 +1273,7 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr, sw = alloc_switch(parent_sw, route, &pkg->ep_uuid); if (!IS_ERR(sw)) { sw->connection_id = pkg->connection_id; - sw->authorized = authorized; + sw->dev.authorized = authorized; sw->security_level = security_level; sw->boot = boot; sw->link_speed = speed_gen3 ? 20 : 10; diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index 3014146081c1..e640d764499a 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -1494,9 +1494,7 @@ static ssize_t authorized_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct tb_switch *sw = tb_to_switch(dev); - - return sprintf(buf, "%u\n", sw->authorized); + return sprintf(buf, "%u\n", dev->authorized); } static int disapprove_switch(struct device *dev, void *not_used) @@ -1505,7 +1503,7 @@ static int disapprove_switch(struct device *dev, void *not_used) struct tb_switch *sw; sw = tb_to_switch(dev); - if (sw && sw->authorized) { + if (sw && sw->dev.authorized) { int ret; /* First children */ @@ -1517,7 +1515,7 @@ static int disapprove_switch(struct device *dev, void *not_used) if (ret) return ret; - sw->authorized = 0; + dev->authorized = false; kobject_uevent_env(&sw->dev.kobj, KOBJ_CHANGE, envp); } @@ -1533,7 +1531,7 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val) if (!mutex_trylock(&sw->tb->lock)) return restart_syscall(); - if (!!sw->authorized == !!val) + if (sw->dev.authorized == !!val) goto unlock; switch (val) { @@ -1564,12 +1562,12 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val) } if (!ret) { - sw->authorized = val; + sw->dev.authorized = !!val; /* * Notify status change to the userspace, informing the new * value of /sys/bus/thunderbolt/devices/.../authorized. */ - sprintf(envp_string, "AUTHORIZED=%u", sw->authorized); + sprintf(envp_string, "AUTHORIZED=%u", sw->dev.authorized); kobject_uevent_env(&sw->dev.kobj, KOBJ_CHANGE, envp); } @@ -1671,7 +1669,7 @@ static ssize_t key_store(struct device *dev, struct device_attribute *attr, if (!mutex_trylock(&sw->tb->lock)) return restart_syscall(); - if (sw->authorized) { + if (sw->dev.authorized) { ret = -EBUSY; } else { kfree(sw->key); @@ -2192,7 +2190,7 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent, /* Root switch is always authorized */ if (!route) - sw->authorized = true; + sw->dev.authorized = true; device_initialize(&sw->dev); sw->dev.parent = parent; diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c index 2897a77d44c3..44d2fa893fa9 100644 --- a/drivers/thunderbolt/tb.c +++ b/drivers/thunderbolt/tb.c @@ -1323,7 +1323,7 @@ static int tb_scan_finalize_switch(struct device *dev, void *data) * send uevent to userspace. */ if (sw->boot) - sw->authorized = 1; + sw->dev.authorized = true; dev_set_uevent_suppress(dev, false); kobject_uevent(&dev->kobj, KOBJ_ADD); diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h index 725104c83e3d..cfe869d8e826 100644 --- a/drivers/thunderbolt/tb.h +++ b/drivers/thunderbolt/tb.h @@ -130,7 +130,6 @@ struct tb_switch_tmu { * @safe_mode: The switch is in safe-mode * @boot: Whether the switch was already authorized on boot or not * @rpm: The switch supports runtime PM - * @authorized: Whether the switch is authorized by user or policy * @security_level: Switch supported security level * @debugfs_dir: Pointer to the debugfs structure * @key: Contains the key used to challenge the device or %NULL if not @@ -180,7 +179,6 @@ struct tb_switch { bool safe_mode; bool boot; bool rpm; - unsigned int authorized; enum tb_security_level security_level; struct dentry *debugfs_dir; u8 *key; diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 072968c40ade..fb476665f52d 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -331,7 +331,7 @@ static int usb_probe_interface(struct device *dev) if (usb_device_is_owned(udev)) return error; - if (udev->authorized == 0) { + if (udev->dev.authorized == false) { dev_err(&intf->dev, "Device is not authorized for usage\n"); return error; } else if (intf->authorized == 0) { diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c index 26f9fb9f67ca..7fa4ca77fa89 100644 --- a/drivers/usb/core/generic.c +++ b/drivers/usb/core/generic.c @@ -230,7 +230,7 @@ int usb_generic_driver_probe(struct usb_device *udev) /* Choose and set the configuration. This registers the interfaces * with the driver core and lets interface drivers bind to them. */ - if (udev->authorized == 0) + if (udev->dev.authorized == false) dev_err(&udev->dev, "Device is not authorized for usage\n"); else { c = usb_choose_configuration(udev); diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 86658a81d284..f58b19aa4f5f 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2616,10 +2616,10 @@ int usb_new_device(struct usb_device *udev) int usb_deauthorize_device(struct usb_device *usb_dev) { usb_lock_device(usb_dev); - if (usb_dev->authorized == 0) + if (usb_dev->dev.authorized == false) goto out_unauthorized; - usb_dev->authorized = 0; + usb_dev->dev.authorized = false; usb_set_configuration(usb_dev, -1); out_unauthorized: @@ -2633,7 +2633,7 @@ int usb_authorize_device(struct usb_device *usb_dev) int result = 0, c; usb_lock_device(usb_dev); - if (usb_dev->authorized == 1) + if (usb_dev->dev.authorized == true) goto out_authorized; result = usb_autoresume_device(usb_dev); @@ -2652,7 +2652,7 @@ int usb_authorize_device(struct usb_device *usb_dev) } } - usb_dev->authorized = 1; + usb_dev->dev.authorized = true; /* Choose and set the configuration. This registers the interfaces * with the driver core and lets interface drivers bind to them. */ diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 4d59d927ae3e..47548ce1cfb1 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1962,7 +1962,7 @@ int usb_set_configuration(struct usb_device *dev, int configuration) struct usb_hcd *hcd = bus_to_hcd(dev->bus); int n, nintf; - if (dev->authorized == 0 || configuration == -1) + if (dev->dev.authorized == false || configuration == -1) configuration = 0; else { for (i = 0; i < dev->descriptor.bNumConfigurations; i++) { diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index fa2e49d432ff..3d63e345d0a0 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -726,8 +726,7 @@ usb_descriptor_attr(bMaxPacketSize0, "%d\n"); static ssize_t authorized_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct usb_device *usb_dev = to_usb_device(dev); - return snprintf(buf, PAGE_SIZE, "%u\n", usb_dev->authorized); + return snprintf(buf, PAGE_SIZE, "%u\n", dev->authorized); } /* diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 62368c4ed37a..18f3ad39ccbc 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -579,6 +579,14 @@ static unsigned usb_bus_is_wusb(struct usb_bus *bus) return hcd->wireless; } +/* + * usb_dev_authorized() - Used to initialize the "authorized" status of + * the USB device. + * (user space) policy determines if we authorize this device to be + * used or not. By default, wired USB devices are authorized. + * WUSB devices are not, until we authorize them from user space. + * FIXME -- complete doc + */ static bool usb_dev_authorized(struct usb_device *dev, struct usb_hcd *hcd) { struct usb_hub *hub; @@ -717,7 +725,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, dev->active_duration = -jiffies; #endif - dev->authorized = usb_dev_authorized(dev, usb_hcd); + dev->dev.authorized = usb_dev_authorized(dev, usb_hcd); if (!root_hub) dev->wusb = usb_bus_is_wusb(bus) ? 1 : 0; diff --git a/include/linux/device.h b/include/linux/device.h index e270cb740b9e..899be9a2c0cb 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -443,7 +443,7 @@ struct dev_links_info { * @removable: Whether the device can be removed from the system. This * should be set by the subsystem / bus driver that discovered * the device. - * + * @authorized: Whether the device is authorized to bind to a driver. * @offline_disabled: If set, the device is permanently online. * @offline: Set after successful invocation of bus type's .offline(). * @of_node_reused: Set if the device-tree node is shared with an ancestor @@ -562,6 +562,7 @@ struct device { enum device_removable removable; + bool authorized:1; bool offline_disabled:1; bool offline:1; bool of_node_reused:1; diff --git a/include/linux/usb.h b/include/linux/usb.h index 7ccaa76a9a96..796df4068e94 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -576,11 +576,6 @@ struct usb3_lpm_parameters { * @can_submit: URBs may be submitted * @persist_enabled: USB_PERSIST enabled for this device * @have_langid: whether string_langid is valid - * @authorized: policy has said we can use it; - * (user space) policy determines if we authorize this device to be - * used or not. By default, wired USB devices are authorized. - * WUSB devices are not, until we authorize them from user space. - * FIXME -- complete doc * @authenticated: Crypto authentication passed * @wusb: device is Wireless USB * @lpm_capable: device supports LPM @@ -662,7 +657,6 @@ struct usb_device { unsigned can_submit:1; unsigned persist_enabled:1; unsigned have_langid:1; - unsigned authorized:1; unsigned authenticated:1; unsigned wusb:1; unsigned lpm_capable:1;