Re: [PATCH v1] USB: sisusbvga: avoid initializing device in open

[Michal Pecio <michal.pecio@gmail.com>]

On Sun,  6 Sep 2026 01:06:34 +0530, Ayush wrote:
> The USB core holds minor_rwsem while invoking a USB character
> device's open callback. sisusb_open() can currently initialize the
> device while holding sisusb->lock, and that initialization performs
> synchronous USB I/O.
> 
> High-speed devices are already initialized during probe. Treat a
> probe-time initialization failure as a probe failure instead of
> retrying initialization from open, so the open callback no longer
> performs blocking device initialization.
> 
> Reported-by: syzbot+3bc656a9271e7c8a5c6b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3bc656a9271e7c8a5c6b

Does this even fix the repro? I see no syzbot test requests there.

It seems that even if you delete this code, sisusb_open() will simply
wait on mutex_lock() before it even reaches this point.

The root cause is isusb_init_gfxdevice(), called by probe() and from
here, taking 10 minutes to complete on nonresponsive device. This is
the second time (that I know of) syzbot ran into this recently.


Thomas, do you still have this HW and a will to improve the driver,
or test / comment on patches? I suspect that those timeouts and retries
in the init routine are vastly excessive.

> Signed-off-by: Ayush <ayush37735@gmail.com>
> ---
>  drivers/usb/misc/sisusbvga/sisusbvga.c | 29 ++++++++++++--------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/misc/sisusbvga/sisusbvga.c b/drivers/usb/misc/sisusbvga/sisusbvga.c
> index 3e75a7c24828..6723a65b12b8 100644
> --- a/drivers/usb/misc/sisusbvga/sisusbvga.c
> +++ b/drivers/usb/misc/sisusbvga/sisusbvga.c
> @@ -2223,20 +2223,10 @@ static int sisusb_open(struct inode *inode, struct file *file)
>  	}
>  
>  	if (!sisusb->devinit) {
> -		if (sisusb->sisusb_dev->speed == USB_SPEED_HIGH ||
> -				sisusb->sisusb_dev->speed >= USB_SPEED_SUPER) {
> -			if (sisusb_init_gfxdevice(sisusb, 0)) {
> -				mutex_unlock(&sisusb->lock);
> -				dev_err(&sisusb->sisusb_dev->dev,
> -						"Failed to initialize device\n");
> -				return -EIO;
> -			}
> -		} else {
> -			mutex_unlock(&sisusb->lock);
> -			dev_err(&sisusb->sisusb_dev->dev,
> -					"Device not attached to USB 2.0 hub\n");
> -			return -EIO;
> -		}
> +		mutex_unlock(&sisusb->lock);
> +		dev_err(&sisusb->sisusb_dev->dev,
> +			"Device not initialized\n");
> +		return -EIO;

Not sure if we really want to log every failure to open some device.

Not sure if EIO is the right code to retun from open(), particularly
after probe failure.

And really, are we opening the device after probe failed (see below)?
Seems weird...

>  	}
>  
>  	/* Increment usage count for our sisusb */
> @@ -2880,9 +2870,16 @@ static int sisusb_probe(struct usb_interface *intf,
>  
>  	if (dev->speed == USB_SPEED_HIGH || dev->speed >= USB_SPEED_SUPER) {
>  		int initscreen = 1;
> -		if (sisusb_init_gfxdevice(sisusb, initscreen))
> +
> +		if (sisusb_init_gfxdevice(sisusb, initscreen)) {
>  			dev_err(&sisusb->sisusb_dev->dev,
> -					"Failed to early initialize device\n");
> +				"Failed to early initialize device\n");
> +			sisusb->present = 0;
> +			usb_set_intfdata(intf, NULL);
> +			usb_put_dev(sisusb->sisusb_dev);
> +			retval = -EIO;
> +			goto error_4;
> +		}
>  
>  	} else
>  		dev_info(&sisusb->sisusb_dev->dev,
> -- 
> 2.53.0
>