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

[Bastien Nocera <hadess@hadess.net>]

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.