| Message ID | 20231208123119.1.If27eb3bf7812f91ab83810f232292f032f4203e0@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: core: Fix crash w/ usb_choose_configuration() if no driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Dec 08, 2023 at 12:31:24PM -0800, Douglas Anderson wrote: > It's possible that usb_choose_configuration() can get called when a > USB device has no driver. In this case the recent commit a87b8e3be926 > ("usb: core: Allow subclassed USB drivers to override > usb_choose_configuration()") can cause a crash since it dereferenced > the driver structure without checking for NULL. Let's add a check. > > This was seen in the real world when usbguard got ahold of a r8152 > device at the wrong time. It can also be simulated via this on a > computer with one r8152-based USB Ethernet adapter: > cd /sys/bus/usb/drivers/r8152-cfgselector > to_unbind="$(ls -d *-*)" > real_dir="$(readlink -f "${to_unbind}")" > echo "${to_unbind}" > unbind > cd "${real_dir}" > echo 0 > authorized > echo 1 > authorized > > Fixes: a87b8e3be926 ("usb: core: Allow subclassed USB drivers to override usb_choose_configuration()") > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- I'm not sure this is the best solution. A USB device with no driver is an anomaly; in all likelihood we shouldn't be calling usb_choose_configuration() for such a device in the first place. So I think a better solution would be to put this check in usb_authorize_device() before it does the autoresume, or else to make usb_choose_configuration() return immediately, right at the start, if there is no driver. Alan Stern
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c index dcb897158228..365482347333 100644 --- a/drivers/usb/core/generic.c +++ b/drivers/usb/core/generic.c @@ -59,15 +59,19 @@ int usb_choose_configuration(struct usb_device *udev) int num_configs; int insufficient_power = 0; struct usb_host_config *c, *best; - struct usb_device_driver *udriver = to_usb_device_driver(udev->dev.driver); + struct usb_device_driver *udriver; if (usb_device_is_owned(udev)) return 0; - if (udriver->choose_configuration) { - i = udriver->choose_configuration(udev); - if (i >= 0) - return i; + if (udev->dev.driver) { + udriver = to_usb_device_driver(udev->dev.driver); + + if (udriver->choose_configuration) { + i = udriver->choose_configuration(udev); + if (i >= 0) + return i; + } } best = NULL;
It's possible that usb_choose_configuration() can get called when a USB device has no driver. In this case the recent commit a87b8e3be926 ("usb: core: Allow subclassed USB drivers to override usb_choose_configuration()") can cause a crash since it dereferenced the driver structure without checking for NULL. Let's add a check. This was seen in the real world when usbguard got ahold of a r8152 device at the wrong time. It can also be simulated via this on a computer with one r8152-based USB Ethernet adapter: cd /sys/bus/usb/drivers/r8152-cfgselector to_unbind="$(ls -d *-*)" real_dir="$(readlink -f "${to_unbind}")" echo "${to_unbind}" > unbind cd "${real_dir}" echo 0 > authorized echo 1 > authorized Fixes: a87b8e3be926 ("usb: core: Allow subclassed USB drivers to override usb_choose_configuration()") Signed-off-by: Douglas Anderson <dianders@chromium.org> --- drivers/usb/core/generic.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)