Re: [PATCH v2 5/6] USB: core: Add API to change the wireless_status

[Alan Stern <stern@rowland.harvard.edu>]

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.

Alan

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.