isdn/gigaset: fix bas_gigaset AT read error handling

Rework the handling of USB errors in AT response reads
to fix a possible infinite retry loop and a memory leak,
and silence a few overly verbose kernel messages.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
CC: stable <stable@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index e143050..131976d 100644 (file)
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -438,23 +438,27 @@ static void cmd_in_timeout(unsigned long data)
                return;
        }
 
-       if (ucs->retry_cmd_in++ < BAS_RETRY) {
-               dev_notice(cs->dev, "control read: timeout, retry %d\n",
-                          ucs->retry_cmd_in);
-               rc = atread_submit(cs, BAS_TIMEOUT);
-               if (rc >= 0 || rc == -ENODEV)
-                       /* resubmitted or disconnected */
-                       /* - bypass regular exit block */
-                       return;
-       } else {
+       if (ucs->retry_cmd_in++ >= BAS_RETRY) {
                dev_err(cs->dev,
                        "control read: timeout, giving up after %d tries\n",
                        ucs->retry_cmd_in);
+               kfree(ucs->rcvbuf);
+               ucs->rcvbuf = NULL;
+               ucs->rcvbuf_size = 0;
+               error_reset(cs);
+               return;
+       }
+
+       gig_dbg(DEBUG_USBREQ, "%s: timeout, retry %d",
+               __func__, ucs->retry_cmd_in);
+       rc = atread_submit(cs, BAS_TIMEOUT);
+       if (rc < 0) {
+               kfree(ucs->rcvbuf);
+               ucs->rcvbuf = NULL;
+               ucs->rcvbuf_size = 0;
+               if (rc != -ENODEV)
+                       error_reset(cs);
        }
-       kfree(ucs->rcvbuf);
-       ucs->rcvbuf = NULL;
-       ucs->rcvbuf_size = 0;
-       error_reset(cs);
 }
 
 /* read_ctrl_callback
@@ -470,18 +474,11 @@ static void read_ctrl_callback(struct urb *urb)
        struct cardstate *cs = inbuf->cs;
        struct bas_cardstate *ucs = cs->hw.bas;
        int status = urb->status;
-       int have_data = 0;
        unsigned numbytes;
        int rc;
 
        update_basstate(ucs, 0, BS_ATRDPEND);
        wake_up(&ucs->waitqueue);
-
-       if (!ucs->rcvbuf_size) {
-               dev_warn(cs->dev, "%s: no receive in progress\n", __func__);
-               return;
-       }
-
        del_timer(&ucs->timer_cmd_in);
 
        switch (status) {
@@ -495,19 +492,10 @@ static void read_ctrl_callback(struct urb *urb)
                                numbytes = ucs->rcvbuf_size;
                }
 
-               /* copy received bytes to inbuf */
-               have_data = gigaset_fill_inbuf(inbuf, ucs->rcvbuf, numbytes);
-
-               if (unlikely(numbytes < ucs->rcvbuf_size)) {
-                       /* incomplete - resubmit for remaining bytes */
-                       ucs->rcvbuf_size -= numbytes;
-                       ucs->retry_cmd_in = 0;
-                       rc = atread_submit(cs, BAS_TIMEOUT);
-                       if (rc >= 0 || rc == -ENODEV)
-                               /* resubmitted or disconnected */
-                               /* - bypass regular exit block */
-                               return;
-                       error_reset(cs);
+               /* copy received bytes to inbuf, notify event layer */
+               if (gigaset_fill_inbuf(inbuf, ucs->rcvbuf, numbytes)) {
+                       gig_dbg(DEBUG_INTR, "%s-->BH", __func__);
+                       gigaset_schedule_event(cs);
                }
                break;
 
@@ -516,37 +504,32 @@ static void read_ctrl_callback(struct urb *urb)
        case -EINPROGRESS:              /* pending */
        case -ENODEV:                   /* device removed */
        case -ESHUTDOWN:                /* device shut down */
-               /* no action necessary */
+               /* no further action necessary */
                gig_dbg(DEBUG_USBREQ, "%s: %s",
                        __func__, get_usb_statmsg(status));
                break;
 
-       default:                        /* severe trouble */
-               dev_warn(cs->dev, "control read: %s\n",
-                        get_usb_statmsg(status));
+       default:                        /* other errors: retry */
                if (ucs->retry_cmd_in++ < BAS_RETRY) {
-                       dev_notice(cs->dev, "control read: retry %d\n",
-                                  ucs->retry_cmd_in);
+                       gig_dbg(DEBUG_USBREQ, "%s: %s, retry %d", __func__,
+                               get_usb_statmsg(status), ucs->retry_cmd_in);
                        rc = atread_submit(cs, BAS_TIMEOUT);
-                       if (rc >= 0 || rc == -ENODEV)
-                               /* resubmitted or disconnected */
-                               /* - bypass regular exit block */
+                       if (rc >= 0)
+                               /* successfully resubmitted, skip freeing */
                                return;
-               } else {
-                       dev_err(cs->dev,
-                               "control read: giving up after %d tries\n",
-                               ucs->retry_cmd_in);
+                       if (rc == -ENODEV)
+                               /* disconnect, no further action necessary */
+                               break;
                }
+               dev_err(cs->dev, "control read: %s, giving up after %d tries\n",
+                       get_usb_statmsg(status), ucs->retry_cmd_in);
                error_reset(cs);
        }
 
+       /* read finished, free buffer */
        kfree(ucs->rcvbuf);
        ucs->rcvbuf = NULL;
        ucs->rcvbuf_size = 0;
-       if (have_data) {
-               gig_dbg(DEBUG_INTR, "%s-->BH", __func__);
-               gigaset_schedule_event(cs);
-       }
 }
 
 /* atread_submit