USB: yurex: Don't retry on unexpected errors
authorAlan Stern <stern@rowland.harvard.edu>
Tue, 17 Sep 2019 16:47:23 +0000 (12:47 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 4 Oct 2019 09:02:59 +0000 (11:02 +0200)
According to Greg KH, it has been generally agreed that when a USB
driver encounters an unknown error (or one it can't handle directly),
it should just give up instead of going into a potentially infinite
retry loop.

The three codes -EPROTO, -EILSEQ, and -ETIME fall into this category.
They can be caused by bus errors such as packet loss or corruption,
attempting to communicate with a disconnected device, or by malicious
firmware.  Nowadays the extent of packet loss or corruption is
negligible, so it should be safe for a driver to give up whenever one
of these errors occurs.

Although the yurex driver handles -EILSEQ errors in this way, it
doesn't do the same for -EPROTO (as discovered by the syzbot fuzzer)
or other unrecognized errors.  This patch adjusts the driver so that
it doesn't log an error message for -EPROTO or -ETIME, and it doesn't
retry after any errors.

Reported-and-tested-by: syzbot+b24d736f18a1541ad550@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>
CC: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.1909171245410.1590-100000@iolanthe.rowland.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/misc/yurex.c

index 6715a12..8d52d43 100644 (file)
@@ -132,6 +132,7 @@ static void yurex_interrupt(struct urb *urb)
        switch (status) {
        case 0: /*success*/
                break;
+       /* The device is terminated or messed up, give up */
        case -EOVERFLOW:
                dev_err(&dev->interface->dev,
                        "%s - overflow with length %d, actual length is %d\n",
@@ -140,12 +141,13 @@ static void yurex_interrupt(struct urb *urb)
        case -ENOENT:
        case -ESHUTDOWN:
        case -EILSEQ:
-               /* The device is terminated, clean up */
+       case -EPROTO:
+       case -ETIME:
                return;
        default:
                dev_err(&dev->interface->dev,
                        "%s - unknown status received: %d\n", __func__, status);
-               goto exit;
+               return;
        }
 
        /* handle received message */
@@ -177,7 +179,6 @@ static void yurex_interrupt(struct urb *urb)
                break;
        }
 
-exit:
        retval = usb_submit_urb(dev->urb, GFP_ATOMIC);
        if (retval) {
                dev_err(&dev->interface->dev, "%s - usb_submit_urb failed: %d\n",