[PATCH] gspca: Kill URBs on USB device disconnect

[PATCH] gspca: Kill URBs on USB device disconnect

[Ezequiel Garcia]

In order to prevent ISOC URBs from being infinitely resubmitted,
the driver's USB disconnect handler must kill all the in-flight URBs.

While here, change the URB packet status message to a debug level,
to avoid spamming the console too much.

This commit fixes a lockup caused by an interrupt storm coming
from the URB completion handler.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/usb/gspca/gspca.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 9448ac0b8bc9..0510c20273aa 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -294,7 +294,7 @@ static void fill_frame(struct gspca_dev *gspca_dev,
 		/* check the packet status and length */
 		st = urb->iso_frame_desc[i].status;
 		if (st) {
-			pr_err("ISOC data error: [%d] len=%d, status=%d\n",
+			gspca_dbg(gspca_dev, D_PACK, "ISOC data error: [%d] len=%d, status=%d\n",
 			       i, len, st);
 			gspca_dev->last_packet_type = DISCARD_PACKET;
 			continue;
@@ -1642,6 +1642,7 @@ void gspca_disconnect(struct usb_interface *intf)
 
 	mutex_lock(&gspca_dev->usb_lock);
 	gspca_dev->present = false;
+	destroy_urbs(gspca_dev);
 
 	vb2_queue_error(&gspca_dev->queue);
 
-- 
2.20.1

[PATCH v2] gspca: Kill URBs on USB device disconnect

[Ezequiel Garcia]

In order to prevent ISOC URBs from being infinitely resubmitted,
the driver's USB disconnect handler must kill all the in-flight URBs.

While here, change the URB packet status message to a debug level,
to avoid spamming the console too much.

This commit fixes a lockup caused by an interrupt storm coming
from the URB completion handler.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
v2:
  * Also kill the int URB.

 drivers/media/usb/gspca/gspca.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 9448ac0b8bc9..4d7517411cc2 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -294,7 +294,7 @@ static void fill_frame(struct gspca_dev *gspca_dev,
 		/* check the packet status and length */
 		st = urb->iso_frame_desc[i].status;
 		if (st) {
-			pr_err("ISOC data error: [%d] len=%d, status=%d\n",
+			gspca_dbg(gspca_dev, D_PACK, "ISOC data error: [%d] len=%d, status=%d\n",
 			       i, len, st);
 			gspca_dev->last_packet_type = DISCARD_PACKET;
 			continue;
@@ -1642,6 +1642,8 @@ void gspca_disconnect(struct usb_interface *intf)
 
 	mutex_lock(&gspca_dev->usb_lock);
 	gspca_dev->present = false;
+	destroy_urbs(gspca_dev);
+	gspca_input_destroy_urb(gspca_dev);
 
 	vb2_queue_error(&gspca_dev->queue);
 
-- 
2.20.1

Re: [PATCH v2] gspca: Kill URBs on USB device disconnect

[Ezequiel Garcia]

On Thu, 2019-02-28 at 12:28 -0300, Ezequiel Garcia wrote:
> In order to prevent ISOC URBs from being infinitely resubmitted,
> the driver's USB disconnect handler must kill all the in-flight URBs.
> 
> While here, change the URB packet status message to a debug level,
> to avoid spamming the console too much.
> 
> This commit fixes a lockup caused by an interrupt storm coming
> from the URB completion handler.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> v2:
>   * Also kill the int URB.
> 

Hans,

We still have to solve another race in this driver,
but I think this fix is good to go.

Thanks,
Eze

>  drivers/media/usb/gspca/gspca.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
> index 9448ac0b8bc9..4d7517411cc2 100644
> --- a/drivers/media/usb/gspca/gspca.c
> +++ b/drivers/media/usb/gspca/gspca.c
> @@ -294,7 +294,7 @@ static void fill_frame(struct gspca_dev *gspca_dev,
>  		/* check the packet status and length */
>  		st = urb->iso_frame_desc[i].status;
>  		if (st) {
> -			pr_err("ISOC data error: [%d] len=%d, status=%d\n",
> +			gspca_dbg(gspca_dev, D_PACK, "ISOC data error: [%d] len=%d, status=%d\n",
>  			       i, len, st);
>  			gspca_dev->last_packet_type = DISCARD_PACKET;
>  			continue;
> @@ -1642,6 +1642,8 @@ void gspca_disconnect(struct usb_interface *intf)
>  
>  	mutex_lock(&gspca_dev->usb_lock);
>  	gspca_dev->present = false;
> +	destroy_urbs(gspca_dev);
> +	gspca_input_destroy_urb(gspca_dev);
>  
>  	vb2_queue_error(&gspca_dev->queue);
>