| Message ID | 20230301122310.3579-5-hadess@hadess.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/6] HID: logitech-hidpp: Simplify array length check | expand |
On Wed, 2023-03-01 at 10:33 -0500, Alan Stern wrote: > On Wed, Mar 01, 2023 at 01:23:09PM +0100, Bastien Nocera wrote: > > This adds the API that allows device specific drivers to tell user- > > space > > about whether the wireless device is connected to its receiver > > dongle. > > > > See "USB: core: Add wireless_status sysfs attribute" for a detailed > > explanation of what this attribute should be used for. > > > > Signed-off-by: Bastien Nocera <hadess@hadess.net> > > --- > > Fixed locking/use-after-free in v2, thanks to Alan Stern > > > > drivers/usb/core/message.c | 40 > > ++++++++++++++++++++++++++++++++++++++ > > include/linux/usb.h | 5 +++++ > > 2 files changed, 45 insertions(+) > > > > diff --git a/drivers/usb/core/message.c > > b/drivers/usb/core/message.c > > index 127fac1af676..3867d9a85145 100644 > > --- a/drivers/usb/core/message.c > > +++ b/drivers/usb/core/message.c > > @@ -1908,6 +1908,45 @@ static void __usb_queue_reset_device(struct > > work_struct *ws) > > usb_put_intf(iface); /* Undo _get_ in > > usb_queue_reset_device() */ > > } > > > > +/* > > + * Internal function to set the wireless_status sysfs attribute > > + * See usb_set_wireless_status() for more details > > + */ > > +static void __usb_wireless_status_intf(struct work_struct *ws) > > +{ > > + struct usb_interface *iface = > > + container_of(ws, struct usb_interface, > > wireless_status_work); > > + > > + device_lock(iface->dev.parent); > > + if (iface->sysfs_files_created) > > + usb_update_wireless_status_attr(iface); > > + usb_put_intf(iface); /* Undo _get_ in > > usb_set_wireless_status() */ > > + device_unlock(iface->dev.parent); > > Whoops! Calling usb_put_intf() means the iface pointer is no longer > valid. The device_unlock() call should come before it, not after. Fixed this in v3. > PS: You might also want to edit the sysfs documentation in the > preceding > patch, to make sure the text doesn't extend beyond the 80-column > limit. > I know people don't pay too much attention to that restriction in > code > any more, but in documentation it helps to keep the lines fairly > short. > People have trouble reading text when the lines get too long. That's an easy fix, so done in v3 as well.
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 127fac1af676..3867d9a85145 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1908,6 +1908,45 @@ static void __usb_queue_reset_device(struct work_struct *ws) usb_put_intf(iface); /* Undo _get_ in usb_queue_reset_device() */ } +/* + * Internal function to set the wireless_status sysfs attribute + * See usb_set_wireless_status() for more details + */ +static void __usb_wireless_status_intf(struct work_struct *ws) +{ + struct usb_interface *iface = + container_of(ws, struct usb_interface, wireless_status_work); + + device_lock(iface->dev.parent); + if (iface->sysfs_files_created) + usb_update_wireless_status_attr(iface); + usb_put_intf(iface); /* Undo _get_ in usb_set_wireless_status() */ + device_unlock(iface->dev.parent); +} + +/** + * usb_set_wireless_status - sets the wireless_status struct member + * @dev: the device to modify + * @status: the new wireless status + * + * Set the wireless_status struct member to the new value, and emit + * sysfs changes as necessary. + * + * Returns: 0 on success, -EALREADY if already set. + */ +int usb_set_wireless_status(struct usb_interface *iface, + enum usb_wireless_status status) +{ + if (iface->wireless_status == status) + return -EALREADY; + + usb_get_intf(iface); + iface->wireless_status = status; + schedule_work(&iface->wireless_status_work); + + return 0; +} +EXPORT_SYMBOL_GPL(usb_set_wireless_status); /* * usb_set_configuration - Makes a particular device setting be current @@ -2100,6 +2139,7 @@ int usb_set_configuration(struct usb_device *dev, int configuration) intf->dev.type = &usb_if_device_type; intf->dev.groups = usb_interface_groups; INIT_WORK(&intf->reset_ws, __usb_queue_reset_device); + INIT_WORK(&intf->wireless_status_work, __usb_wireless_status_intf); intf->minor = -1; device_initialize(&intf->dev); pm_runtime_no_callbacks(&intf->dev); diff --git a/include/linux/usb.h b/include/linux/usb.h index 46fc85aba0df..a48eeec62a66 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -262,6 +262,7 @@ struct usb_interface { unsigned resetting_device:1; /* true: bandwidth alloc after reset */ unsigned authorized:1; /* used for interface authorization */ enum usb_wireless_status wireless_status; + struct work_struct wireless_status_work; struct device dev; /* interface specific device info */ struct device *usb_dev; @@ -896,6 +897,10 @@ static inline int usb_interface_claimed(struct usb_interface *iface) extern void usb_driver_release_interface(struct usb_driver *driver, struct usb_interface *iface); + +int usb_set_wireless_status(struct usb_interface *iface, + enum usb_wireless_status status); + const struct usb_device_id *usb_match_id(struct usb_interface *interface, const struct usb_device_id *id); extern int usb_match_one_id(struct usb_interface *interface,
This adds the API that allows device specific drivers to tell user-space about whether the wireless device is connected to its receiver dongle. See "USB: core: Add wireless_status sysfs attribute" for a detailed explanation of what this attribute should be used for. Signed-off-by: Bastien Nocera <hadess@hadess.net> --- Fixed locking/use-after-free in v2, thanks to Alan Stern drivers/usb/core/message.c | 40 ++++++++++++++++++++++++++++++++++++++ include/linux/usb.h | 5 +++++ 2 files changed, 45 insertions(+)