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

[Alan Stern]

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.

And if the order in the registration error path does not matter, why 
does the patch change it?

Or did you mean something else?

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.

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?

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.  
:-)

Alan Stern

> Fixes: 765e0ba62613 ("usb-serial: new API for driver registration")
> Cc: stable@xxxxxxxxxxxxxxx	# 3.4
> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
>  drivers/usb/serial/usb-serial.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> index a4fbc849c0fa..08a9b6f8cf7d 100644
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -1522,9 +1522,9 @@ int __usb_serial_register_drivers(struct usb_serial_driver *const serial_drivers
>  	return 0;
>  
>  err_deregister_drivers:
> +	usb_deregister(udriver);
>  	while (sd-- > serial_drivers)
>  		usb_serial_deregister(*sd);
> -	usb_deregister(udriver);
>  err_free_driver:
>  	kfree(udriver);
>  	return rc;
> @@ -1543,9 +1543,15 @@ void usb_serial_deregister_drivers(struct usb_serial_driver *const serial_driver
>  {
>  	struct usb_driver *udriver = (*serial_drivers)->usb_driver;
>  
> +	/*
> +	 * The USB driver must be deregistered before the USB serial drivers
> +	 * so that I/O is stopped before unbinding the ports.
> +	 */
> +	usb_deregister(udriver);
> +
>  	for (; *serial_drivers; ++serial_drivers)
>  		usb_serial_deregister(*serial_drivers);
> -	usb_deregister(udriver);
> +
>  	kfree(udriver);
>  }
>  EXPORT_SYMBOL_GPL(usb_serial_deregister_drivers);
> -- 
> 2.54.0
>