[PATCH v2] USB: sisusbvga: avoid prolonged initialization on failure

[PATCH v2] USB: sisusbvga: avoid prolonged initialization on failure

[Ayush Yaduvanshi]

The USB core holds minor_rwsem while invoking a USB character
device's open callback. Avoid retrying graphics-device initialization
from open() and fail probe when early initialization fails.

Also stop sisusb_do_init_gfxdevice() after the first failed request.
Continuing initialization after a failed USB request causes additional
synchronous transfers and their retries against an unresponsive device.

v2:
Stop sisusb_do_init_gfxdevice() after the first failed USB request and
treat early initialization failure in probe() as a probe failure.
Remove the initialization retry from sisusb_open().

The exact v2 patch was tested by syzbot using the reported reproducer,
and the reproducer did not trigger the reported hang.

Reported-by: syzbot+3bc656a9271e7c8a5c6b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3bc656a9271e7c8a5c6b
Tested-by: syzbot+3bc656a9271e7c8a5c6b@syzkaller.appspotmail.com
Signed-off-by: Ayush Yaduvanshi <ayush37735@gmail.com>
---
Thanks for the review.

v2:
- Stop sisusb_do_init_gfxdevice() after the first failed USB request
  instead of continuing with further synchronous requests and their
  timeout/retry cost on an unresponsive device.
- Treat failure of the early graphics-device initialization in probe()
  as a probe failure, so a device which failed initialization is not
  left for a later initialization attempt from open().
- Remove the graphics-device initialization retry from sisusb_open().
- The exact v2 patch was tested by syzbot using the reported reproducer;
  syzbot reports that the reproducer did not trigger the reported hang.

 drivers/usb/misc/sisusbvga/sisusbvga.c | 133 +++++++++++++++++--------
 1 file changed, 89 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusbvga.c b/drivers/usb/misc/sisusbvga/sisusbvga.c
index 3e75a7c24828..e0b1111c1f53 100644
--- a/drivers/usb/misc/sisusbvga/sisusbvga.c
+++ b/drivers/usb/misc/sisusbvga/sisusbvga.c
@@ -2075,66 +2075,114 @@ static int sisusb_do_init_gfxdevice(struct sisusb_usb_data *sisusb)
 	packet.address = 0x00000324;
 	packet.data    = 0x00000004;
 	ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	if (ret)
+		return ret;
 
 	packet.header  = 0x001f;
 	packet.address = 0x00000364;
 	packet.data    = 0x00000004;
-	ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	if (ret)
+		return ret;
 
 	packet.header  = 0x001f;
 	packet.address = 0x00000384;
 	packet.data    = 0x00000004;
-	ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	if (ret)
+		return ret;
 
 	packet.header  = 0x001f;
 	packet.address = 0x00000100;
 	packet.data    = 0x00000700;
-	ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	if (ret)
+		return ret;
 
 	packet.header  = 0x000f;
 	packet.address = 0x00000004;
-	ret |= sisusb_send_bridge_packet(sisusb, 6, &packet, 0);
+	ret = sisusb_send_bridge_packet(sisusb, 6, &packet, 0);
+	if (ret)
+		return ret;
+
 	packet.data |= 0x17;
-	ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
+	if (ret)
+		return ret;
 
 	/* Init BAR 0 (VRAM) */
-	ret |= sisusb_read_pci_config(sisusb, 0x10, &tmp32);
-	ret |= sisusb_write_pci_config(sisusb, 0x10, 0xfffffff0);
-	ret |= sisusb_read_pci_config(sisusb, 0x10, &tmp32);
+	ret = sisusb_read_pci_config(sisusb, 0x10, &tmp32);
+	if (ret)
+		return ret;
+
+	ret = sisusb_write_pci_config(sisusb, 0x10, 0xfffffff0);
+	if (ret)
+		return ret;
+
+	ret = sisusb_read_pci_config(sisusb, 0x10, &tmp32);
+	if (ret)
+		return ret;
+
 	tmp32 &= 0x0f;
 	tmp32 |= SISUSB_PCI_MEMBASE;
-	ret |= sisusb_write_pci_config(sisusb, 0x10, tmp32);
+	ret = sisusb_write_pci_config(sisusb, 0x10, tmp32);
+	if (ret)
+		return ret;
 
 	/* Init BAR 1 (MMIO) */
-	ret |= sisusb_read_pci_config(sisusb, 0x14, &tmp32);
-	ret |= sisusb_write_pci_config(sisusb, 0x14, 0xfffffff0);
-	ret |= sisusb_read_pci_config(sisusb, 0x14, &tmp32);
+	ret = sisusb_read_pci_config(sisusb, 0x14, &tmp32);
+	if (ret)
+		return ret;
+
+	ret = sisusb_write_pci_config(sisusb, 0x14, 0xfffffff0);
+	if (ret)
+		return ret;
+
+	ret = sisusb_read_pci_config(sisusb, 0x14, &tmp32);
+	if (ret)
+		return ret;
+
 	tmp32 &= 0x0f;
 	tmp32 |= SISUSB_PCI_MMIOBASE;
-	ret |= sisusb_write_pci_config(sisusb, 0x14, tmp32);
+	ret = sisusb_write_pci_config(sisusb, 0x14, tmp32);
+	if (ret)
+		return ret;
 
 	/* Init BAR 2 (i/o ports) */
-	ret |= sisusb_read_pci_config(sisusb, 0x18, &tmp32);
-	ret |= sisusb_write_pci_config(sisusb, 0x18, 0xfffffff0);
-	ret |= sisusb_read_pci_config(sisusb, 0x18, &tmp32);
+	ret = sisusb_read_pci_config(sisusb, 0x18, &tmp32);
+	if (ret)
+		return ret;
+
+	ret = sisusb_write_pci_config(sisusb, 0x18, 0xfffffff0);
+	if (ret)
+		return ret;
+
+	ret = sisusb_read_pci_config(sisusb, 0x18, &tmp32);
+	if (ret)
+		return ret;
+
 	tmp32 &= 0x0f;
 	tmp32 |= SISUSB_PCI_IOPORTBASE;
-	ret |= sisusb_write_pci_config(sisusb, 0x18, tmp32);
+	ret = sisusb_write_pci_config(sisusb, 0x18, tmp32);
+	if (ret)
+		return ret;
 
 	/* Enable memory and i/o access */
-	ret |= sisusb_read_pci_config(sisusb, 0x04, &tmp32);
+	ret = sisusb_read_pci_config(sisusb, 0x04, &tmp32);
+	if (ret)
+		return ret;
+
 	tmp32 |= 0x3;
-	ret |= sisusb_write_pci_config(sisusb, 0x04, tmp32);
-
-	if (ret == 0) {
-		/* Some further magic */
-		packet.header  = 0x001f;
-		packet.address = 0x00000050;
-		packet.data    = 0x000000ff;
-		ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
-	}
+	ret = sisusb_write_pci_config(sisusb, 0x04, tmp32);
+	if (ret)
+		return ret;
 
-	return ret;
+	/* Some further magic */
+	packet.header  = 0x001f;
+	packet.address = 0x00000050;
+	packet.data    = 0x000000ff;
+
+	return sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
 }
 
 /* Initialize the graphics device (return 0 on success)
@@ -2223,20 +2271,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;
 	}
 
 	/* Increment usage count for our sisusb */
@@ -2880,9 +2918,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

Re: [PATCH v2] USB: sisusbvga: avoid prolonged initialization on failure

[Ayush Yaduvanshi]

Hi Michal,

Just a quick follow-up on v2.

Syzbot tested the exact v2 patch with the reported reproducer and reported that
the reproducer did not trigger the reported hang.

Please let me know if any further changes or testing are needed.

Thanks,
Ayush Yaduvanshi

On Sun, Sep 20, 2026 at 12:00 AM Ayush Yaduvanshi <ayush37735@gmail.com> wrote:
>
> Hi Michal,
>
> Just a quick follow-up on v2.
>
> Syzbot tested the exact v2 patch with the reported reproducer and reported that
> the reproducer did not trigger the reported hang.
>
> Please let me know if any further changes or testing are needed.
>
> Thanks,
> Ayush Yaduvanshi
>
> On Mon, Sep 7, 2026 at 1:13 PM Ayush Yaduvanshi <ayush37735@gmail.com> wrote:
>>
>> The USB core holds minor_rwsem while invoking a USB character
>> device's open callback. Avoid retrying graphics-device initialization
>> from open() and fail probe when early initialization fails.
>>
>> Also stop sisusb_do_init_gfxdevice() after the first failed request.
>> Continuing initialization after a failed USB request causes additional
>> synchronous transfers and their retries against an unresponsive device.
>>
>> v2:
>> Stop sisusb_do_init_gfxdevice() after the first failed USB request and
>> treat early initialization failure in probe() as a probe failure.
>> Remove the initialization retry from sisusb_open().
>>
>> The exact v2 patch was tested by syzbot using the reported reproducer,
>> and the reproducer did not trigger the reported hang.
>>
>> Reported-by: syzbot+3bc656a9271e7c8a5c6b@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=3bc656a9271e7c8a5c6b
>> Tested-by: syzbot+3bc656a9271e7c8a5c6b@syzkaller.appspotmail.com
>> Signed-off-by: Ayush Yaduvanshi <ayush37735@gmail.com>
>> ---
>> Thanks for the review.
>>
>> v2:
>> - Stop sisusb_do_init_gfxdevice() after the first failed USB request
>>   instead of continuing with further synchronous requests and their
>>   timeout/retry cost on an unresponsive device.
>> - Treat failure of the early graphics-device initialization in probe()
>>   as a probe failure, so a device which failed initialization is not
>>   left for a later initialization attempt from open().
>> - Remove the graphics-device initialization retry from sisusb_open().
>> - The exact v2 patch was tested by syzbot using the reported reproducer;
>>   syzbot reports that the reproducer did not trigger the reported hang.
>>
>>  drivers/usb/misc/sisusbvga/sisusbvga.c | 133 +++++++++++++++++--------
>>  1 file changed, 89 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/usb/misc/sisusbvga/sisusbvga.c b/drivers/usb/misc/sisusbvga/sisusbvga.c
>> index 3e75a7c24828..e0b1111c1f53 100644
>> --- a/drivers/usb/misc/sisusbvga/sisusbvga.c
>> +++ b/drivers/usb/misc/sisusbvga/sisusbvga.c
>> @@ -2075,66 +2075,114 @@ static int sisusb_do_init_gfxdevice(struct sisusb_usb_data *sisusb)
>>         packet.address = 0x00000324;
>>         packet.data    = 0x00000004;
>>         ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       if (ret)
>> +               return ret;
>>
>>         packet.header  = 0x001f;
>>         packet.address = 0x00000364;
>>         packet.data    = 0x00000004;
>> -       ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       if (ret)
>> +               return ret;
>>
>>         packet.header  = 0x001f;
>>         packet.address = 0x00000384;
>>         packet.data    = 0x00000004;
>> -       ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       if (ret)
>> +               return ret;
>>
>>         packet.header  = 0x001f;
>>         packet.address = 0x00000100;
>>         packet.data    = 0x00000700;
>> -       ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       if (ret)
>> +               return ret;
>>
>>         packet.header  = 0x000f;
>>         packet.address = 0x00000004;
>> -       ret |= sisusb_send_bridge_packet(sisusb, 6, &packet, 0);
>> +       ret = sisusb_send_bridge_packet(sisusb, 6, &packet, 0);
>> +       if (ret)
>> +               return ret;
>> +
>>         packet.data |= 0x17;
>> -       ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       ret = sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> +       if (ret)
>> +               return ret;
>>
>>         /* Init BAR 0 (VRAM) */
>> -       ret |= sisusb_read_pci_config(sisusb, 0x10, &tmp32);
>> -       ret |= sisusb_write_pci_config(sisusb, 0x10, 0xfffffff0);
>> -       ret |= sisusb_read_pci_config(sisusb, 0x10, &tmp32);
>> +       ret = sisusb_read_pci_config(sisusb, 0x10, &tmp32);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = sisusb_write_pci_config(sisusb, 0x10, 0xfffffff0);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = sisusb_read_pci_config(sisusb, 0x10, &tmp32);
>> +       if (ret)
>> +               return ret;
>> +
>>         tmp32 &= 0x0f;
>>         tmp32 |= SISUSB_PCI_MEMBASE;
>> -       ret |= sisusb_write_pci_config(sisusb, 0x10, tmp32);
>> +       ret = sisusb_write_pci_config(sisusb, 0x10, tmp32);
>> +       if (ret)
>> +               return ret;
>>
>>         /* Init BAR 1 (MMIO) */
>> -       ret |= sisusb_read_pci_config(sisusb, 0x14, &tmp32);
>> -       ret |= sisusb_write_pci_config(sisusb, 0x14, 0xfffffff0);
>> -       ret |= sisusb_read_pci_config(sisusb, 0x14, &tmp32);
>> +       ret = sisusb_read_pci_config(sisusb, 0x14, &tmp32);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = sisusb_write_pci_config(sisusb, 0x14, 0xfffffff0);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = sisusb_read_pci_config(sisusb, 0x14, &tmp32);
>> +       if (ret)
>> +               return ret;
>> +
>>         tmp32 &= 0x0f;
>>         tmp32 |= SISUSB_PCI_MMIOBASE;
>> -       ret |= sisusb_write_pci_config(sisusb, 0x14, tmp32);
>> +       ret = sisusb_write_pci_config(sisusb, 0x14, tmp32);
>> +       if (ret)
>> +               return ret;
>>
>>         /* Init BAR 2 (i/o ports) */
>> -       ret |= sisusb_read_pci_config(sisusb, 0x18, &tmp32);
>> -       ret |= sisusb_write_pci_config(sisusb, 0x18, 0xfffffff0);
>> -       ret |= sisusb_read_pci_config(sisusb, 0x18, &tmp32);
>> +       ret = sisusb_read_pci_config(sisusb, 0x18, &tmp32);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = sisusb_write_pci_config(sisusb, 0x18, 0xfffffff0);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = sisusb_read_pci_config(sisusb, 0x18, &tmp32);
>> +       if (ret)
>> +               return ret;
>> +
>>         tmp32 &= 0x0f;
>>         tmp32 |= SISUSB_PCI_IOPORTBASE;
>> -       ret |= sisusb_write_pci_config(sisusb, 0x18, tmp32);
>> +       ret = sisusb_write_pci_config(sisusb, 0x18, tmp32);
>> +       if (ret)
>> +               return ret;
>>
>>         /* Enable memory and i/o access */
>> -       ret |= sisusb_read_pci_config(sisusb, 0x04, &tmp32);
>> +       ret = sisusb_read_pci_config(sisusb, 0x04, &tmp32);
>> +       if (ret)
>> +               return ret;
>> +
>>         tmp32 |= 0x3;
>> -       ret |= sisusb_write_pci_config(sisusb, 0x04, tmp32);
>> -
>> -       if (ret == 0) {
>> -               /* Some further magic */
>> -               packet.header  = 0x001f;
>> -               packet.address = 0x00000050;
>> -               packet.data    = 0x000000ff;
>> -               ret |= sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>> -       }
>> +       ret = sisusb_write_pci_config(sisusb, 0x04, tmp32);
>> +       if (ret)
>> +               return ret;
>>
>> -       return ret;
>> +       /* Some further magic */
>> +       packet.header  = 0x001f;
>> +       packet.address = 0x00000050;
>> +       packet.data    = 0x000000ff;
>> +
>> +       return sisusb_send_bridge_packet(sisusb, 10, &packet, 0);
>>  }
>>
>>  /* Initialize the graphics device (return 0 on success)
>> @@ -2223,20 +2271,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;
>>         }
>>
>>         /* Increment usage count for our sisusb */
>> @@ -2880,9 +2918,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
>>

Re: [PATCH v2] USB: sisusbvga: avoid prolonged initialization on failure

[Ayush Yaduvanshi]

Hi Greg,

Friendly ping on this v2 patch. It fixes a task hang reported by
syzbot in sisusbvga.

The patch was tested and verified by syzbot using the reported
reproducer, confirming that the hang is resolved. Could you please
review and consider it for usb-next when you have a moment?

Patch link: https://lore.kernel.org/linux-usb/20260907074308.109603-1-ayush37735@gmail.com/
Syzbot bug: https://syzkaller.appspot.com/bug?extid=3bc656a9271e7c8a5c6b

Thanks,
Ayush Yaduvanshi

Re: [PATCH v2] USB: sisusbvga: avoid prolonged initialization on failure

[Greg Kroah-Hartman]

On Wed, Oct 14, 2026 at 03:32:18PM +0530, Ayush Yaduvanshi wrote:
> Hi Greg,
> 
> Friendly ping on this v2 patch. It fixes a task hang reported by
> syzbot in sisusbvga.

I have no context here, why did you delete it all?

> The patch was tested and verified by syzbot using the reported
> reproducer, confirming that the hang is resolved. Could you please
> review and consider it for usb-next when you have a moment?
> 
> Patch link: https://lore.kernel.org/linux-usb/20260907074308.109603-1-ayush37735@gmail.com/
> Syzbot bug: https://syzkaller.appspot.com/bug?extid=3bc656a9271e7c8a5c6b

I have 2000+ linux-usb patches/messages to review right now.  Please
help out by doing reviews of other submissions so that yours moves to
the top of the queue.

And for syzbot issues, for fake devices like this (i.e. no real device
hits this), it's _WAY_ down the priority list for obvious reasons (i.e.
no user will ever trigger this.)

thanks,

greg k-h