On Fri, 17 May 2019, syzbot wrote: > Hello, > > syzbot tried to test the proposed patch but build/boot failed: Drat. Mistake in the patch. Let's try again. Incidentally, as far as I can tell there's no point in having the usb_get_dev() in p54u_probe() and usb_put_dev() in p54u_disconnect(). The device structure is guaranteed not to be deallocated while a driver is bound to any of its interfaces, so taking an extra reference won't make any difference. On the other hand, I do see some problems in the firmware-load callback. First, it calls device_release_driver() without first checking that the interface is still bound to the p54u driver. Second, it shouldn't call device_release_driver() at all -- it should call usb_driver_release_interface(). It doesn't want to unbind the USB device's driver; it wants to unbind the interface's driver. And third, to do this it needs to acquire udev's device lock, not the lock for udev's parent. Alan Stern #syz test: https://github.com/google/kasan.git usb-fuzzer drivers/net/wireless/intersil/p54/p54usb.c | 3 +++ 1 file changed, 3 insertions(+) Index: usb-devel/drivers/net/wireless/intersil/p54/p54usb.c =================================================================== --- usb-devel.orig/drivers/net/wireless/intersil/p54/p54usb.c +++ usb-devel/drivers/net/wireless/intersil/p54/p54usb.c @@ -923,6 +923,7 @@ static void p54u_load_firmware_cb(const struct usb_device *udev = priv->udev; int err; + pr_info("%s: priv->udev = %px\n", __func__, udev); complete(&priv->fw_wait_load); if (firmware) { priv->fw = firmware; @@ -969,6 +970,8 @@ static int p54u_load_firmware(struct iee if (i < 0) return i; + dev_info(&udev->dev, "%s: udev @ %px, dev.parent @ %px\n", + __func__, udev, &udev->dev.parent); dev_info(&priv->udev->dev, "Loading firmware file %s\n", p54u_fwlist[i].fw);
