Re: [PATCH] USB: core: don't release an unbound claimed interface via the driver core

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: core: incomplete fix allows KASAN null-ptr-deref via sysfs unbind
--

commit fd7979038e7e5e8a14bb9739dc4ce0f3b1576847
Author: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

USB: core: don't release an unbound claimed interface via the driver core

usb_driver_claim_interface() sets dev->driver on an interface and only
calls device_bind_driver() if the interface is already registered.
Otherwise the later device_add() is expected to bind it, but with
bus drivers_autoprobe disabled bus_probe_device() skips the attach, so
the interface ends up with dev->driver set while it was never added to
the driver's klist.

Closes: https://syzkaller.appspot.com/bug?extid=863936f50214e843ae0c

> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 3c3bbaf89609a..20e8e61bf6713 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -636,10 +636,11 @@ void usb_driver_release_interface(struct usb_driver *driver,
>  		return;
>  	iface->condition = USB_INTERFACE_UNBINDING;
>  
> -	/* Release via the driver core only if the interface
> -	 * has already been registered
> +	/* Release via the driver core only if the interface has been
> +	 * bound by it.  A claimed interface is not bound if it was
> +	 * registered with drivers_autoprobe disabled.
>  	 */
> -	if (device_is_registered(dev)) {
> +	if (device_is_bound(dev)) {

[Severity: High]
Does this change fully resolve the issue, or does the crash still trigger
via the sysfs unbind path?

If drivers_autoprobe=0 and the CDC NCM control interface is manually bound,
usb_driver_claim_interface() sets dev->driver for the data interface without
actually binding it, leaving its klist node unattached.

This patch prevents the crash in usb_driver_release_interface() by checking
device_is_bound(), but what happens if an administrator writes the data
interface's device name to /sys/bus/usb/drivers/cdc_ncm/unbind?

The unbind_store() function in drivers/base/bus.c will call
device_driver_detach(), reaching __device_release_driver():

drivers/base/dd.c:__device_release_driver() {
        ...
        if (dev->driver) {
                ...
                klist_remove(&dev->p->knode_driver);
                ...
}

Since dev->driver is set, will this still attempt to remove the unattached
klist node and result in the same NULL pointer dereference in klist_put()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008003431.145352-1-ngocthang2710.1999@gmail.com?part=1