Skip to content

Commit 43ea236

Browse files
rhodaszigregkh
authored andcommitted
usb: cdc-wdm: avoid setting WDM_READ for ZLP-s
[ Upstream commit 387602d ] 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 <[email protected]> Acked-by: Oliver Neukum <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent 53809d3 commit 43ea236

File tree

1 file changed

+9
-14
lines changed

1 file changed

+9
-14
lines changed

drivers/usb/class/cdc-wdm.c

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ struct wdm_device {
9292
u16 wMaxCommand;
9393
u16 wMaxPacketSize;
9494
__le16 inum;
95-
int reslength;
9695
int length;
9796
int read;
9897
int count;
@@ -214,6 +213,11 @@ static void wdm_in_callback(struct urb *urb)
214213
if (desc->rerr == 0 && status != -EPIPE)
215214
desc->rerr = status;
216215

216+
if (length == 0) {
217+
dev_dbg(&desc->intf->dev, "received ZLP\n");
218+
goto skip_zlp;
219+
}
220+
217221
if (length + desc->length > desc->wMaxCommand) {
218222
/* The buffer would overflow */
219223
set_bit(WDM_OVERFLOW, &desc->flags);
@@ -222,18 +226,18 @@ static void wdm_in_callback(struct urb *urb)
222226
if (!test_bit(WDM_OVERFLOW, &desc->flags)) {
223227
memmove(desc->ubuf + desc->length, desc->inbuf, length);
224228
desc->length += length;
225-
desc->reslength = length;
226229
}
227230
}
228231
skip_error:
229232

230233
if (desc->rerr) {
231234
/*
232-
* Since there was an error, userspace may decide to not read
233-
* any data after poll'ing.
235+
* If there was a ZLP or an error, userspace may decide to not
236+
* read any data after poll'ing.
234237
* We should respond to further attempts from the device to send
235238
* data, so that we can get unstuck.
236239
*/
240+
skip_zlp:
237241
schedule_work(&desc->service_outs_intr);
238242
} else {
239243
set_bit(WDM_READ, &desc->flags);
@@ -585,15 +589,6 @@ static ssize_t wdm_read
585589
goto retry;
586590
}
587591

588-
if (!desc->reslength) { /* zero length read */
589-
dev_dbg(&desc->intf->dev, "zero length - clearing WDM_READ\n");
590-
clear_bit(WDM_READ, &desc->flags);
591-
rv = service_outstanding_interrupt(desc);
592-
spin_unlock_irq(&desc->iuspin);
593-
if (rv < 0)
594-
goto err;
595-
goto retry;
596-
}
597592
cntr = desc->length;
598593
spin_unlock_irq(&desc->iuspin);
599594
}
@@ -1016,7 +1011,7 @@ static void service_interrupt_work(struct work_struct *work)
10161011

10171012
spin_lock_irq(&desc->iuspin);
10181013
service_outstanding_interrupt(desc);
1019-
if (!desc->resp_count) {
1014+
if (!desc->resp_count && (desc->length || desc->rerr)) {
10201015
set_bit(WDM_READ, &desc->flags);
10211016
wake_up(&desc->wait);
10221017
}

0 commit comments

Comments
 (0)