usb: cdc-wdm: avoid setting WDM_READ for ZLP-s

Don't set WDM_READ flag in wdm_in_callback() for ZLP-s, otherwise when
userspace tries to poll for available data, it might - incorrectly -
believe there is something available, and when it tries to non-blocking
read it, it might get stuck in the read loop.

For example this is what glib does for non-blocking read (briefly):

  1. poll()
  2. if poll returns with non-zero, starts a read data loop:
    a. loop on poll() (EINTR disabled)
    b. if revents was set, reads data
      I. if read returns with EINTR or EAGAIN, goto 2.a.
      II. otherwise return with data

So if ZLP sets WDM_READ (#1), we expect data, and try to read it (#2).
But as that was a ZLP, and we are doing non-blocking read, wdm_read()
returns with EAGAIN (#2.b.I), so loop again, and try to read again
(#2.a.).

With glib, we might stuck in this loop forever, as EINTR is disabled
(#2.a).

Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com>
Acked-by: Oliver Neukum <oneukum@suse.com>
Link: https://lore.kernel.org/r/20250403144004.3889125-1-robert.hodaszi@digi.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 86ee39db013f3932cf96635ddbde3a86b5c9309a..28c5ed840a4096ebfa0d5b580b54684e02a64530 100644 (file)
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -92,7 +92,6 @@ struct wdm_device {
        u16                     wMaxCommand;
        u16                     wMaxPacketSize;
        __le16                  inum;
-       int                     reslength;
        int                     length;
        int                     read;
        int                     count;
@@ -214,6 +213,11 @@ static void wdm_in_callback(struct urb *urb)
        if (desc->rerr == 0 && status != -EPIPE)
                desc->rerr = status;
 
+       if (length == 0) {
+               dev_dbg(&desc->intf->dev, "received ZLP\n");
+               goto skip_zlp;
+       }
+
        if (length + desc->length > desc->wMaxCommand) {
                /* The buffer would overflow */
                set_bit(WDM_OVERFLOW, &desc->flags);
@@ -222,18 +226,18 @@ static void wdm_in_callback(struct urb *urb)
                if (!test_bit(WDM_OVERFLOW, &desc->flags)) {
                        memmove(desc->ubuf + desc->length, desc->inbuf, length);
                        desc->length += length;
-                       desc->reslength = length;
                }
        }
 skip_error:
 
        if (desc->rerr) {
                /*
-                * Since there was an error, userspace may decide to not read
-                * any data after poll'ing.
+                * If there was a ZLP or an error, userspace may decide to not
+                * read any data after poll'ing.
                 * We should respond to further attempts from the device to send
                 * data, so that we can get unstuck.
                 */
+skip_zlp:
                schedule_work(&desc->service_outs_intr);
        } else {
                set_bit(WDM_READ, &desc->flags);
@@ -585,15 +589,6 @@ retry:
                        goto retry;
                }
 
-               if (!desc->reslength) { /* zero length read */
-                       dev_dbg(&desc->intf->dev, "zero length - clearing WDM_READ\n");
-                       clear_bit(WDM_READ, &desc->flags);
-                       rv = service_outstanding_interrupt(desc);
-                       spin_unlock_irq(&desc->iuspin);
-                       if (rv < 0)
-                               goto err;
-                       goto retry;
-               }
                cntr = desc->length;
                spin_unlock_irq(&desc->iuspin);
        }
@@ -1005,7 +1000,7 @@ static void service_interrupt_work(struct work_struct *work)
 
        spin_lock_irq(&desc->iuspin);
        service_outstanding_interrupt(desc);
-       if (!desc->resp_count) {
+       if (!desc->resp_count && (desc->length || desc->rerr)) {
                set_bit(WDM_READ, &desc->flags);
                wake_up(&desc->wait);
        }