Re: [PATCH 2/2] USB: serial: fix driver deregistration order

[Johan Hovold <johan@kernel.org>]

On Thu, Aug 20, 2026 at 11:47:05AM -0400, Alan Stern wrote:
> On Thu, Aug 20, 2026 at 04:54:02PM +0200, Johan Hovold wrote:
> > USB serial driver modules register one driver for the USB bus and one or
> > more drivers for the ports on the USB serial bus.
> > 
> > When unloading a driver module, the USB driver must be deregistered
> > before the USB serial bus drivers so that I/O is stopped before
> > unbinding the ports to avoid use-after-free in completion handlers
> > accessing port data.
> > 
> > Note that the order does not matter currently in the registration error
> > path as the USB driver is not bound until after the USB serial drivers
> > have been registered.
> 
> But __usb_serial_register_drivers() does usb_register(udriver) _before_ 
> calling usb_serial_register() for the serial_drivers.  Not _after_, as 
> claimed here.

It registers the driver, but it doesn't set up the id table and
therefore will not bind to any interface until after the serial drivers
have also been registered (as an optimisation).
 
> And if the order in the registration error path does not matter, why 
> does the patch change it?

For symmetry with deregistration and for the unlikely event that there
is ever a later error path added (or if the optimisation mentioned above
is removed).

> Also, what about the failure mode described in 765e0ba62613?  Not to 
> mention that it seems odd to register a parent device driver after a 
> child device driver, rather than before.  Or to unregister them in the 
> reverse order.

There is generally no issue registering a driver for a child device
after a driver for a parent so that everything is in place when the
parent driver registers the child device.

In fact, that is sort of what is done today by not setting up the
id_table until after the serial drivers have been registered even if
things are complicated by how dynamic id support is implemented for USB
serial.

As I'm not changing the registration order I sort of skimmed over the
dynamic id issue addressed by commit 765e0ba62613 ("usb-serial: new API
for driver registration"). But I guess need to revisit that to make sure
I don't reintroduce a race during deregistration.
 
> If the point is to avoid I/O operations completing after the device 
> structure has been deallocated, why not rely on the serial-bus drivers 
> to stop all their I/O when they are unbound?

The problem is that there are two drivers involved for the USB interface
and USB serial port, respectively, both of which can do I/O and where
the parent USB driver may access the driver data of the child ports.

Tearing things down properly is a bit involved as you know (and as can
be seen from the first patch in the series). Everything is therefore
managed from the parent driver disconnect callback and individual ports
are, for example, no longer allowed to be unbound independently (cf.
commit fdb838efa31e ("USB: serial: suppress driver bind attributes")).

The only remaining corner case should be driver deregistration.

> If these questions don't make sense, remember that I haven't done 
> serious work on this subsystem for 15 years or so and blame it on that.  
> :-)

Heh. They make perfect sense. Thanks for taking a look.

Johan