[PATCH] si470x: fixup error handling of the interrupt URB

From: Oliver Neukum
Date: Wed Dec 04 2019 - 07:40:19 EST


The error handling of the interrupt URB is not correct
in every case and assumes that low level errors
are either transient or end with a disconnect.

Starting IO to a device is not necessarily a NOP in every error
case. So we need to terminate all IO in every case of probe
failure and disconnect with absolute certainty.
We also must not retry forever in an error case.
As this is unlikely in an actual device, we just give
up.

Reported-and-tested-by: syzbot+9ca7a12fd736d93e0232@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
---
drivers/media/radio/si470x/radio-si470x-usb.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c
index fedff68d8c49..1b974c2683a6 100644
--- a/drivers/media/radio/si470x/radio-si470x-usb.c
+++ b/drivers/media/radio/si470x/radio-si470x-usb.c
@@ -370,15 +370,12 @@ static void si470x_int_in_callback(struct urb *urb)
unsigned char tmpbuf[3];

if (urb->status) {
- if (urb->status == -ENOENT ||
+ if (!(urb->status == -ENOENT ||
urb->status == -ECONNRESET ||
- urb->status == -ESHUTDOWN) {
- return;
- } else {
+ urb->status == -ESHUTDOWN))
dev_warn(&radio->intf->dev,
"non-zero urb status (%d)\n", urb->status);
- goto resubmit; /* Maybe we can recover. */
- }
+ return;
}

/* Sometimes the device returns len 0 packets */
@@ -463,6 +460,7 @@ static void si470x_int_in_callback(struct urb *urb)
/* Resubmit if we're still running. */
if (radio->int_in_running && radio->usbdev) {
retval = usb_submit_urb(radio->int_in_urb, GFP_ATOMIC);
+ printk(KERN_ERR"In resubmit code path with result %d\n", retval);
if (retval) {
dev_warn(&radio->intf->dev,
"resubmitting urb failed (%d)", retval);
@@ -542,6 +540,8 @@ static int si470x_start_usb(struct si470x_device *radio)
radio->int_in_running = 0;
}
radio->status_rssi_auto_update = radio->int_in_running;
+ if (retval < 0)
+ return retval;

/* start radio */
retval = si470x_start(radio);
@@ -734,7 +734,8 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
/* start radio */
retval = si470x_start_usb(radio);
if (retval < 0)
- goto err_buf;
+ /* the urb may be running even after an error */
+ goto err_all;

/* set initial frequency */
si470x_set_freq(radio, 87.5 * FREQ_MUL); /* available in all regions */
@@ -749,8 +750,7 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,

return 0;
err_all:
- usb_kill_urb(radio->int_in_urb);
-err_buf:
+ usb_poison_urb(radio->int_in_urb);
kfree(radio->buffer);
err_ctrl:
v4l2_ctrl_handler_free(&radio->hdl);
@@ -824,7 +824,7 @@ static void si470x_usb_driver_disconnect(struct usb_interface *intf)
mutex_lock(&radio->lock);
v4l2_device_disconnect(&radio->v4l2_dev);
video_unregister_device(&radio->videodev);
- usb_kill_urb(radio->int_in_urb);
+ usb_poison_urb(radio->int_in_urb);
usb_set_intfdata(intf, NULL);
mutex_unlock(&radio->lock);
v4l2_device_put(&radio->v4l2_dev);
--
2.16.4