usb: wwan: Avoid RX URB submission races
Since commit 6f806b8fa362b53f10947c65af2df8d4acb10b61 "usb: serial:
Add flow control between wwan and tty drivers", pushing of received
URB data into TTY has been offloaded into a work queue. Only after
the data is transferred will the URB be resubmitted. This can race
with usb_wwan_resume, which unconditionally submits all the available
IN URBs. Hence the double call to usb_anchor_urb will corrupt the
anchor list, the second submission will fail, and the following
usb_unanchor_urb will result in the list pointing to a now unlinked
URB. Thus portdata->in_urb_list will never be exhausted as it can
never be traversed fully, and the work function will keep attempting
to resubmit the problematic URB over and over again.
Fix this by ensuring that an URB to be submitted is both not already
anchored and its urb_list member is not part of a list, so that it
cannot be submitted multiple times. Also, don't allow the work
queue function to resubmit the URB if the interface is suspended.
Change-Id: Ib660d01a502be917297ba02db90dd0e53e241707
CRs-Fixed: 390847
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Neha Pandey <nehap@codeaurora.org>
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index 366df67..0d05e2f 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -283,6 +283,7 @@
{
struct usb_wwan_port_private *portdata =
container_of(w, struct usb_wwan_port_private, in_work);
+ struct usb_wwan_intf_private *intfdata;
struct list_head *q = &portdata->in_urb_list;
struct urb *urb;
unsigned char *data;
@@ -304,7 +305,9 @@
if (!tty)
break;
- list_del_init(&urb->urb_list);
+ /* list_empty() will still be false after this; it means
+ * URB is still being processed */
+ list_del(&urb->urb_list);
spin_unlock_irqrestore(&portdata->in_lock, flags);
@@ -326,18 +329,27 @@
spin_unlock_irqrestore(&portdata->in_lock, flags);
return;
}
+
+ /* re-init list pointer to indicate we are done with it */
+ INIT_LIST_HEAD(&urb->urb_list);
+
portdata->n_read = 0;
+ intfdata = port->serial->private;
- usb_anchor_urb(urb, &portdata->submitted);
- err = usb_submit_urb(urb, GFP_ATOMIC);
- if (err) {
- usb_unanchor_urb(urb);
- if (err != -EPERM)
- pr_err("%s: submit read urb failed:%d",
- __func__, err);
+ spin_lock_irqsave(&intfdata->susp_lock, flags);
+ if (!intfdata->suspended && !urb->anchor) {
+ usb_anchor_urb(urb, &portdata->submitted);
+ err = usb_submit_urb(urb, GFP_ATOMIC);
+ if (err) {
+ usb_unanchor_urb(urb);
+ if (err != -EPERM)
+ pr_err("%s: submit read urb failed:%d",
+ __func__, err);
+ }
+
+ usb_mark_last_busy(port->serial->dev);
}
-
- usb_mark_last_busy(port->serial->dev);
+ spin_unlock_irqrestore(&intfdata->susp_lock, flags);
spin_lock_irqsave(&portdata->in_lock, flags);
}
spin_unlock_irqrestore(&portdata->in_lock, flags);
@@ -348,6 +360,7 @@
int err;
int endpoint;
struct usb_wwan_port_private *portdata;
+ struct usb_wwan_intf_private *intfdata;
struct usb_serial_port *port;
int status = urb->status;
unsigned long flags;
@@ -357,6 +370,7 @@
endpoint = usb_pipeendpoint(urb->pipe);
port = urb->context;
portdata = usb_get_serial_port_data(port);
+ intfdata = port->serial->private;
usb_mark_last_busy(port->serial->dev);
@@ -373,6 +387,13 @@
dbg("%s: nonzero status: %d on endpoint %02x.",
__func__, status, endpoint);
+ spin_lock(&intfdata->susp_lock);
+ if (intfdata->suspended || !portdata->opened) {
+ spin_unlock(&intfdata->susp_lock);
+ return;
+ }
+ spin_unlock(&intfdata->susp_lock);
+
if (status != -ESHUTDOWN) {
usb_anchor_urb(urb, &portdata->submitted);
err = usb_submit_urb(urb, GFP_ATOMIC);
@@ -849,11 +870,17 @@
for (j = 0; j < N_IN_URB; j++) {
urb = portdata->in_urbs[j];
+
+ /* don't re-submit if it already was submitted or if
+ * it is being processed by in_work */
+ if (urb->anchor || !list_empty(&urb->urb_list))
+ continue;
+
usb_anchor_urb(urb, &portdata->submitted);
err = usb_submit_urb(urb, GFP_ATOMIC);
if (err < 0) {
- err("%s: Error %d for bulk URB %d",
- __func__, err, i);
+ err("%s: Error %d for bulk URB[%d]:%p %d",
+ __func__, err, j, urb, i);
usb_unanchor_urb(urb);
intfdata->suspended = 1;
spin_unlock_irq(&intfdata->susp_lock);