Re: [PATCH v4] USB: Fix device driver race

[Bastien Nocera <hadess@hadess.net>]

On Fri, 2020-07-24 at 13:39 +0300, Andy Shevchenko wrote:
> On Fri, Jul 24, 2020 at 1:32 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net>
> > wrote:
> 
> ...
> 
> > > +static int __usb_bus_reprobe_drivers(struct device *dev, void
> > > *data)
> > > +{
> > > +       struct usb_device_driver *new_udriver = data;
> > > +       struct usb_device_driver *udriver;
> > > +       struct usb_device *udev;
> > > +
> > > +       if (!dev->driver)
> > > +               return 0;
> > > +
> > > +       udriver = to_usb_device_driver(dev->driver);
> > > +       if (udriver != &usb_generic_driver)
> > > +               return 0;
> > 
> > What about
> > 
> > static bool is_dev_usb_generic_driver(dev)
> > {
> >       struct usb_device_driver *udd = dev->driver ?
> > to_usb_device_driver(dev->driver) : NULL;
> > 
> >       return udd == &usb_generic_driver;
> > }
> > 
> >   if (!is_dev_usb_generic_driver)
> >     return 0;
> > 
> > ?
> > 
> > > +       udev = to_usb_device(dev);
> > > +       if (usb_device_match_id(udev, new_udriver->id_table) !=
> > > NULL ||
> > > +           (new_udriver->match && new_udriver->match(udev) ==
> > > 0))
> > > +               device_reprobe(dev);
> > > +
> > > +       return 0;
> > 
> > What about
> > 
> >      udev = to_usb_device(dev);
> >      if (usb_device_match_id(udev, new_udriver->id_table) == NULL)
> >        return 0;
> > ?
> > 
> >   if (new_udriver->match && new_udriver->match(udev) == 0))
> >                device_reprobe(dev);
> >     return 0;
> > 
> > > +}
> 
> It actually sparks a lot of similarities with
> __check_usb_generic(). Perhaps you may unify them?

They might look alike, but apart from that last check (id_table and
match), they're really not. It's a different device driver that's
getting checked against the generic driver.

I tried to merge those, and it ended up being either completely wrong,
or just as long as the code that used to be there before.