Skip to content

Commit 846f10d

Browse files
rhodaszigregkh
authored andcommitted
usb: cdc-wdm: avoid setting WDM_READ for ZLP-s
[ Upstream commit 387602d8a75574fafb451b7a8215e78dfd67ee63 ] 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 65c364c commit 846f10d

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
@@ -89,7 +89,6 @@ struct wdm_device {
8989
u16 wMaxCommand;
9090
u16 wMaxPacketSize;
9191
__le16 inum;
92-
int reslength;
9392
int length;
9493
int read;
9594
int count;
@@ -201,6 +200,11 @@ static void wdm_in_callback(struct urb *urb)
201200
if (desc->rerr == 0 && status != -EPIPE)
202201
desc->rerr = status;
203202

203+
if (length == 0) {
204+
dev_dbg(&desc->intf->dev, "received ZLP\n");
205+
goto skip_zlp;
206+
}
207+
204208
if (length + desc->length > desc->wMaxCommand) {
205209
/* The buffer would overflow */
206210
set_bit(WDM_OVERFLOW, &desc->flags);
@@ -209,18 +213,18 @@ static void wdm_in_callback(struct urb *urb)
209213
if (!test_bit(WDM_OVERFLOW, &desc->flags)) {
210214
memmove(desc->ubuf + desc->length, desc->inbuf, length);
211215
desc->length += length;
212-
desc->reslength = length;
213216
}
214217
}
215218
skip_error:
216219

217220
if (desc->rerr) {
218221
/*
219-
* Since there was an error, userspace may decide to not read
220-
* any data after poll'ing.
222+
* If there was a ZLP or an error, userspace may decide to not
223+
* read any data after poll'ing.
221224
* We should respond to further attempts from the device to send
222225
* data, so that we can get unstuck.
223226
*/
227+
skip_zlp:
224228
schedule_work(&desc->service_outs_intr);
225229
} else {
226230
set_bit(WDM_READ, &desc->flags);
@@ -571,15 +575,6 @@ static ssize_t wdm_read
571575
goto retry;
572576
}
573577

574-
if (!desc->reslength) { /* zero length read */
575-
dev_dbg(&desc->intf->dev, "zero length - clearing WDM_READ\n");
576-
clear_bit(WDM_READ, &desc->flags);
577-
rv = service_outstanding_interrupt(desc);
578-
spin_unlock_irq(&desc->iuspin);
579-
if (rv < 0)
580-
goto err;
581-
goto retry;
582-
}
583578
cntr = desc->length;
584579
spin_unlock_irq(&desc->iuspin);
585580
}
@@ -839,7 +834,7 @@ static void service_interrupt_work(struct work_struct *work)
839834

840835
spin_lock_irq(&desc->iuspin);
841836
service_outstanding_interrupt(desc);
842-
if (!desc->resp_count) {
837+
if (!desc->resp_count && (desc->length || desc->rerr)) {
843838
set_bit(WDM_READ, &desc->flags);
844839
wake_up(&desc->wait);
845840
}

0 commit comments

Comments
 (0)