USB: keyspan: fix NULL-pointer dereferences and memory leaks

Fix NULL-pointer dereference at release by moving port data allocation
and deallocation to port_probe and port_remove.

Fix NULL-pointer dereference at disconnect by stopping port urbs at
port_remove.

Since commit 0998d0631001288 (device-core: Ensure drvdata = NULL when no
driver is bound) the port private data is no longer accessible at
disconnect or release.

Note that this patch also fixes port and interface-data memory leaks in
the error path of attach should port initialisation fail for any port.

Compile-only tested.

Cc: <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

2 files changed, 96 insertions, 93 deletions

diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index 29c943d737d0..7179b0c5f814 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -1374,13 +1374,9 @@ static struct callbacks {
            data in device_details */
 static void keyspan_setup_urbs(struct usb_serial *serial)
 {
-        int                             i, j;
         struct keyspan_serial_private   *s_priv;
         const struct keyspan_device_details     *d_details;
-        struct usb_serial_port          *port;
-        struct keyspan_port_private     *p_priv;
         struct callbacks                *cback;
-        int                             endp;
 
         s_priv = usb_get_serial_data(serial);
         d_details = s_priv->device_details;
@@ -1404,45 +1400,6 @@ static void keyspan_setup_urbs(struct usb_serial *serial)
                 (serial, d_details->glocont_endpoint, USB_DIR_OUT,
                  serial, s_priv->glocont_buf, GLOCONT_BUFLEN,
                  cback->glocont_callback);
-
-        /* Setup endpoints for each port specific thing */
-        for (i = 0; i < d_details->num_ports; i++) {
-                port = serial->port[i];
-                p_priv = usb_get_serial_port_data(port);
-
-                /* Do indat endpoints first, once for each flip */
-                endp = d_details->indat_endpoints[i];
-                for (j = 0; j <= d_details->indat_endp_flip; ++j, ++endp) {
-                        p_priv->in_urbs[j] = keyspan_setup_urb
-                                (serial, endp, USB_DIR_IN, port,
-                                 p_priv->in_buffer[j], 64,
-                                 cback->indat_callback);
-                }
-                for (; j < 2; ++j)
-                        p_priv->in_urbs[j] = NULL;
-
-                /* outdat endpoints also have flip */
-                endp = d_details->outdat_endpoints[i];
-                for (j = 0; j <= d_details->outdat_endp_flip; ++j, ++endp) {
-                        p_priv->out_urbs[j] = keyspan_setup_urb
-                                (serial, endp, USB_DIR_OUT, port,
-                                 p_priv->out_buffer[j], 64,
-                                 cback->outdat_callback);
-                }
-                for (; j < 2; ++j)
-                        p_priv->out_urbs[j] = NULL;
-
-                /* inack endpoint */
-                p_priv->inack_urb = keyspan_setup_urb
-                        (serial, d_details->inack_endpoints[i], USB_DIR_IN,
-                         port, p_priv->inack_buffer, 1, cback->inack_callback);
-
-                /* outcont endpoint */
-                p_priv->outcont_urb = keyspan_setup_urb
-                        (serial, d_details->outcont_endpoints[i], USB_DIR_OUT,
-                         port, p_priv->outcont_buffer, 64,
-                         cback->outcont_callback);
-        }
 }
 
 /* usa19 function doesn't require prescaler */
@@ -2407,9 +2364,7 @@ static void keyspan_send_setup(struct usb_serial_port *port, int reset_port)
 static int keyspan_startup(struct usb_serial *serial)
 {
         int                             i, err;
-        struct usb_serial_port          *port;
         struct keyspan_serial_private   *s_priv;
-        struct keyspan_port_private     *p_priv;
         const struct keyspan_device_details     *d_details;
 
         for (i = 0; (d_details = keyspan_devices[i]) != NULL; ++i)
@@ -2432,19 +2387,6 @@ static int keyspan_startup(struct usb_serial *serial)
         s_priv->device_details = d_details;
         usb_set_serial_data(serial, s_priv);
 
-        /* Now setup per port private data */
-        for (i = 0; i < serial->num_ports; i++) {
-                port = serial->port[i];
-                p_priv = kzalloc(sizeof(struct keyspan_port_private),
-                                                                GFP_KERNEL);
-                if (!p_priv) {
-                        dev_dbg(&port->dev, "%s - kmalloc for keyspan_port_private (%d) failed!.\n", __func__, i);
-                        return 1;
-                }
-                p_priv->device_details = d_details;
-                usb_set_serial_port_data(port, p_priv);
-        }
-
         keyspan_setup_urbs(serial);
 
         if (s_priv->instat_urb != NULL) {
@@ -2463,59 +2405,112 @@ static int keyspan_startup(struct usb_serial *serial)
 
 static void keyspan_disconnect(struct usb_serial *serial)
 {
-        int                             i, j;
-        struct usb_serial_port          *port;
-        struct keyspan_serial_private   *s_priv;
-        struct keyspan_port_private     *p_priv;
+        struct keyspan_serial_private *s_priv;
 
         s_priv = usb_get_serial_data(serial);
 
-        /* Stop reading/writing urbs */
         stop_urb(s_priv->instat_urb);
         stop_urb(s_priv->glocont_urb);
         stop_urb(s_priv->indat_urb);
-        for (i = 0; i < serial->num_ports; ++i) {
-                port = serial->port[i];
-                p_priv = usb_get_serial_port_data(port);
-                stop_urb(p_priv->inack_urb);
-                stop_urb(p_priv->outcont_urb);
-                for (j = 0; j < 2; j++) {
-                        stop_urb(p_priv->in_urbs[j]);
-                        stop_urb(p_priv->out_urbs[j]);
-                }
-        }
+}
+
+static void keyspan_release(struct usb_serial *serial)
+{
+        struct keyspan_serial_private *s_priv;
+
+        s_priv = usb_get_serial_data(serial);
 
-        /* Now free them */
         usb_free_urb(s_priv->instat_urb);
         usb_free_urb(s_priv->indat_urb);
         usb_free_urb(s_priv->glocont_urb);
-        for (i = 0; i < serial->num_ports; ++i) {
-                port = serial->port[i];
-                p_priv = usb_get_serial_port_data(port);
-                usb_free_urb(p_priv->inack_urb);
-                usb_free_urb(p_priv->outcont_urb);
-                for (j = 0; j < 2; j++) {
-                        usb_free_urb(p_priv->in_urbs[j]);
-                        usb_free_urb(p_priv->out_urbs[j]);
-                }
-        }
+
+        kfree(s_priv);
 }
 
-static void keyspan_release(struct usb_serial *serial)
+static int keyspan_port_probe(struct usb_serial_port *port)
 {
-        int                             i;
-        struct usb_serial_port          *port;
-        struct keyspan_serial_private   *s_priv;
+        struct usb_serial *serial = port->serial;
+        struct keyspan_port_private *s_priv;
+        struct keyspan_port_private *p_priv;
+        const struct keyspan_device_details *d_details;
+        struct callbacks *cback;
+        int endp;
+        int port_num;
+        int i;
 
         s_priv = usb_get_serial_data(serial);
+        d_details = s_priv->device_details;
 
-        kfree(s_priv);
+        p_priv = kzalloc(sizeof(*p_priv), GFP_KERNEL);
+        if (!p_priv)
+                return -ENOMEM;
 
-        /* Now free per port private data */
-        for (i = 0; i < serial->num_ports; i++) {
-                port = serial->port[i];
-                kfree(usb_get_serial_port_data(port));
+        s_priv = usb_get_serial_data(port->serial);
+        p_priv->device_details = d_details;
+
+        /* Setup values for the various callback routines */
+        cback = &keyspan_callbacks[d_details->msg_format];
+
+        port_num = port->number - port->serial->minor;
+
+        /* Do indat endpoints first, once for each flip */
+        endp = d_details->indat_endpoints[port_num];
+        for (i = 0; i <= d_details->indat_endp_flip; ++i, ++endp) {
+                p_priv->in_urbs[i] = keyspan_setup_urb(serial, endp,
+                                                USB_DIR_IN, port,
+                                                p_priv->in_buffer[i], 64,
+                                                cback->indat_callback);
+        }
+        /* outdat endpoints also have flip */
+        endp = d_details->outdat_endpoints[port_num];
+        for (i = 0; i <= d_details->outdat_endp_flip; ++i, ++endp) {
+                p_priv->out_urbs[i] = keyspan_setup_urb(serial, endp,
+                                                USB_DIR_OUT, port,
+                                                p_priv->out_buffer[i], 64,
+                                                cback->outdat_callback);
+        }
+        /* inack endpoint */
+        p_priv->inack_urb = keyspan_setup_urb(serial,
+                                        d_details->inack_endpoints[port_num],
+                                        USB_DIR_IN, port,
+                                        p_priv->inack_buffer, 1,
+                                        cback->inack_callback);
+        /* outcont endpoint */
+        p_priv->outcont_urb = keyspan_setup_urb(serial,
+                                        d_details->outcont_endpoints[port_num],
+                                        USB_DIR_OUT, port,
+                                        p_priv->outcont_buffer, 64,
+                                         cback->outcont_callback);
+
+        usb_set_serial_port_data(port, p_priv);
+
+        return 0;
+}
+
+static int keyspan_port_remove(struct usb_serial_port *port)
+{
+        struct keyspan_port_private *p_priv;
+        int i;
+
+        p_priv = usb_get_serial_port_data(port);
+
+        stop_urb(p_priv->inack_urb);
+        stop_urb(p_priv->outcont_urb);
+        for (i = 0; i < 2; i++) {
+                stop_urb(p_priv->in_urbs[i]);
+                stop_urb(p_priv->out_urbs[i]);
+        }
+
+        usb_free_urb(p_priv->inack_urb);
+        usb_free_urb(p_priv->outcont_urb);
+        for (i = 0; i < 2; i++) {
+                usb_free_urb(p_priv->in_urbs[i]);
+                usb_free_urb(p_priv->out_urbs[i]);
         }
+
+        kfree(p_priv);
+
+        return 0;
 }
 
 MODULE_AUTHOR(DRIVER_AUTHOR);
diff --git a/drivers/usb/serial/keyspan.h b/drivers/usb/serial/keyspan.h
index 0a8a40b5711e..0273dda303a4 100644
--- a/drivers/usb/serial/keyspan.h
+++ b/drivers/usb/serial/keyspan.h
@@ -42,6 +42,8 @@ static void keyspan_dtr_rts		(struct usb_serial_port *port, int on);
 static int  keyspan_startup             (struct usb_serial *serial);
 static void keyspan_disconnect          (struct usb_serial *serial);
 static void keyspan_release             (struct usb_serial *serial);
+static int keyspan_port_probe(struct usb_serial_port *port);
+static int keyspan_port_remove(struct usb_serial_port *port);
 static int  keyspan_write_room          (struct tty_struct *tty);
 
 static int  keyspan_write               (struct tty_struct *tty,
@@ -567,6 +569,8 @@ static struct usb_serial_driver keyspan_1port_device = {
         .attach                 = keyspan_startup,
         .disconnect             = keyspan_disconnect,
         .release                = keyspan_release,
+        .port_probe             = keyspan_port_probe,
+        .port_remove            = keyspan_port_remove,
 };
 
 static struct usb_serial_driver keyspan_2port_device = {
@@ -589,6 +593,8 @@ static struct usb_serial_driver keyspan_2port_device = {
         .attach                 = keyspan_startup,
         .disconnect             = keyspan_disconnect,
         .release                = keyspan_release,
+        .port_probe             = keyspan_port_probe,
+        .port_remove            = keyspan_port_remove,
 };
 
 static struct usb_serial_driver keyspan_4port_device = {
@@ -611,6 +617,8 @@ static struct usb_serial_driver keyspan_4port_device = {
         .attach                 = keyspan_startup,
         .disconnect             = keyspan_disconnect,
         .release                = keyspan_release,
+        .port_probe             = keyspan_port_probe,
+        .port_remove            = keyspan_port_remove,
 };
 
 static struct usb_serial_driver * const serial_drivers[] = {