Re: [PATCH] usb: core: don't set drvdata to NULL in usb_unbind_interface()

[Johan Hovold]

On Fri, Oct 02, 2026 at 04:52:49PM +0200, Danilo Krummrich wrote:
> usb_unbind_interface() serves as the remove() callback of struct
> usb_driver and calls usb_set_intfdata(intf, NULL) to clear the bus
> device private data pointer.
> 
> However, the driver core code already sets the bus device private data
> pointer to NULL in device_unbind_cleanup() *after* devres_release_all(),
> which makes the call redundant.
> 
> In addition, it can create unexpected NULL pointer dereference scenarios
> when drivers use managed APIs.
> 
> 	int probe(struct usb_interface *intf,
> 		  const struct usb_device_id *id)
> 	{
> 		struct data *data;
> 		int ret;
> 
> 		data = devm_kzalloc(&intf->dev, sizeof(*data), GFP_KERNEL);
> 		if (!data)
> 			return -ENOMEM;
> 
> 		ret = devm_device_add_group(&intf->dev, &foo_attr_group);
> 		if (ret)
> 			return ret;
> 
> 		...
> 	}
> 
> 	ssize_t foo_value_show(struct device *dev, struct device_attribute *attr,
> 			       char *buf)
> 	{
> 		struct usb_interface *intf = to_usb_interface(dev);
> 		struct data *data = usb_get_intfdata(intf);
> 
> 		/* Potential NULL pointer dereference */
> 		return sysfs_emit(buf, "%u\n", data->value);
> 	}
> 
> Nothing prevents usb_unbind_interface() to race with foo_value_show()
> and set usb_set_intfdata(intf, NULL).

Fortunately, we don't seem to have any USB drivers that use these devres
interfaces. (There is one recent USB HID driver, but it uses static
driver data (!) and clears the driver data pointer itself on unbind...).

> Besides that, the Rust driver core code manages a driver's bus device
> private data and destroys it in device_unbind_cleanup().
> 
> If usb_unbind_interface() sets the pointer to NULL prematurely, the Rust
> driver core code sees NULL, and hence skips the destructor of the bus
> device private data, which leaks all its resources.
> 
> Thus, drop usb_set_intfdata(intf, NULL) from usb_unbind_interface() and
> move it to usb_driver_release_interface(), which manually calls the
> remove() callback of struct usb_driver, and hence can't rely on the
> driver core.

I believe you need to drop the usb_set_intfdata() in the
usb_probe_interface() error path as well.

> Cc: stable@xxxxxxxxxx
> Fixes: a995fe1a3aa7 ("rust: driver: drop device private data post unbind")
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>

With the above fixed you can add my:

Reviewed-by: Johan Hovold <johan@xxxxxxxxxx>

Johan