USB: usb-wwan: fix multiple memory leaks in error paths

commit b8f0e82044c9ba40e92340c8a6d47d6bd6d819bc upstream.

Fix port-data memory leak in usb-serial probe error path by moving port
data allocation to port_probe.

Since commit a1028f0abf ("usb: usb_wwan: replace release and disconnect
with a port_remove hook") port data is deallocated in port_remove. This
leaves a possibility for memory leaks if usb-serial probe fails after
attach but before the port in question has been successfully registered.

Note that this patch also fixes two additional memory leaks in the error
path of attach should port initialisation fail for any port as the urbs
were never freed and neither was the data of any of the successfully
initialised ports.

Compile-only tested.

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

diff --git a/drivers/usb/serial/ipw.c b/drivers/usb/serial/ipw.c
index 2cb30c535839db429c0165a76be26afdf84063cb..dfaf7f1a475b2fcd008925b3fb84ae1b50029e7e 100644 (file)
--- a/drivers/usb/serial/ipw.c
+++ b/drivers/usb/serial/ipw.c
@@ -311,8 +311,8 @@ static struct usb_serial_driver ipw_device = {
        .open =                 ipw_open,
        .close =                ipw_close,
        .probe =                ipw_probe,
-       .attach =               usb_wwan_startup,
        .release =              ipw_release,
+       .port_probe =           usb_wwan_port_probe,
        .port_remove =          usb_wwan_port_remove,
        .dtr_rts =              ipw_dtr_rts,
        .write =                usb_wwan_write,

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index a0542ca03a3fd00d59b306e04aafeb594ebbb2ca..1f33e34addbab64c5c9999e024c4401fdd5bea2d 100644 (file)
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1288,8 +1288,8 @@ static struct usb_serial_driver option_1port_device = {
        .tiocmget          = usb_wwan_tiocmget,
        .tiocmset          = usb_wwan_tiocmset,
        .ioctl             = usb_wwan_ioctl,
-       .attach            = usb_wwan_startup,
        .release           = option_release,
+       .port_probe        = usb_wwan_port_probe,
        .port_remove       = usb_wwan_port_remove,
        .read_int_callback = option_instat_callback,
 #ifdef CONFIG_PM

diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
index bfd50779f0c9f9bf93d2f795322912caa3978f7b..f039733df186b78765256302f9cf545e2f577116 100644 (file)
--- a/drivers/usb/serial/qcserial.c
+++ b/drivers/usb/serial/qcserial.c
@@ -287,8 +287,8 @@ static struct usb_serial_driver qcdevice = {
        .write               = usb_wwan_write,
        .write_room          = usb_wwan_write_room,
        .chars_in_buffer     = usb_wwan_chars_in_buffer,
-       .attach              = usb_wwan_startup,
        .release             = qc_release,
+       .port_probe          = usb_wwan_port_probe,
        .port_remove         = usb_wwan_port_remove,
 #ifdef CONFIG_PM
        .suspend             = usb_wwan_suspend,

diff --git a/drivers/usb/serial/usb-wwan.h b/drivers/usb/serial/usb-wwan.h
index 1f034d2397c6c6ea3888e79aa6cd2ddb6d73202b..684739b8efd05b15a5646585d94417f3aa20a53d 100644 (file)
--- a/drivers/usb/serial/usb-wwan.h
+++ b/drivers/usb/serial/usb-wwan.h
@@ -8,7 +8,7 @@
 extern void usb_wwan_dtr_rts(struct usb_serial_port *port, int on);
 extern int usb_wwan_open(struct tty_struct *tty, struct usb_serial_port *port);
 extern void usb_wwan_close(struct usb_serial_port *port);
-extern int usb_wwan_startup(struct usb_serial *serial);
+extern int usb_wwan_port_probe(struct usb_serial_port *port);
 extern int usb_wwan_port_remove(struct usb_serial_port *port);
 extern int usb_wwan_write_room(struct tty_struct *tty);
 extern void usb_wwan_set_termios(struct tty_struct *tty,

diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index 6855d5ed033115473cf02e3a6f3ee73d1d5bace8..2f2d074a9081e0c00d91a4e6e89826894a1564d1 100644 (file)
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -447,10 +447,12 @@ void usb_wwan_close(struct usb_serial_port *port)
 EXPORT_SYMBOL(usb_wwan_close);
 
 /* Helper functions used by usb_wwan_setup_urbs */
-static struct urb *usb_wwan_setup_urb(struct usb_serial *serial, int endpoint,
+static struct urb *usb_wwan_setup_urb(struct usb_serial_port *port,
+                                     int endpoint,
                                      int dir, void *ctx, char *buf, int len,
                                      void (*callback) (struct urb *))
 {
+       struct usb_serial *serial = port->serial;
        struct urb *urb;
 
        if (endpoint == -1)
@@ -470,100 +472,74 @@ static struct urb *usb_wwan_setup_urb(struct usb_serial *serial, int endpoint,
        return urb;
 }
 
-/* Setup urbs */
-static void usb_wwan_setup_urbs(struct usb_serial *serial)
+int usb_wwan_port_probe(struct usb_serial_port *port)
 {
-       int i, j;
-       struct usb_serial_port *port;
        struct usb_wwan_port_private *portdata;
+       struct urb *urb;
+       u8 *buffer;
+       int err;
+       int i;
 
-       for (i = 0; i < serial->num_ports; i++) {
-               port = serial->port[i];
-               portdata = usb_get_serial_port_data(port);
-
-               /* Do indat endpoints first */
-               for (j = 0; j < N_IN_URB; ++j) {
-                       portdata->in_urbs[j] = usb_wwan_setup_urb(serial,
-                                                                 port->
-                                                                 bulk_in_endpointAddress,
-                                                                 USB_DIR_IN,
-                                                                 port,
-                                                                 portdata->
-                                                                 in_buffer[j],
-                                                                 IN_BUFLEN,
-                                                                 usb_wwan_indat_callback);
-               }
-
-               /* outdat endpoints */
-               for (j = 0; j < N_OUT_URB; ++j) {
-                       portdata->out_urbs[j] = usb_wwan_setup_urb(serial,
-                                                                  port->
-                                                                  bulk_out_endpointAddress,
-                                                                  USB_DIR_OUT,
-                                                                  port,
-                                                                  portdata->
-                                                                  out_buffer
-                                                                  [j],
-                                                                  OUT_BUFLEN,
-                                                                  usb_wwan_outdat_callback);
-               }
-       }
-}
+       portdata = kzalloc(sizeof(*portdata), GFP_KERNEL);
+       if (!portdata)
+               return -ENOMEM;
 
-int usb_wwan_startup(struct usb_serial *serial)
-{
-       int i, j, err;
-       struct usb_serial_port *port;
-       struct usb_wwan_port_private *portdata;
-       u8 *buffer;
+       init_usb_anchor(&portdata->delayed);
 
-       /* Now setup per port private data */
-       for (i = 0; i < serial->num_ports; i++) {
-               port = serial->port[i];
-               portdata = kzalloc(sizeof(*portdata), GFP_KERNEL);
-               if (!portdata) {
-                       dbg("%s: kmalloc for usb_wwan_port_private (%d) failed!.",
-                           __func__, i);
-                       return 1;
-               }
-               init_usb_anchor(&portdata->delayed);
+       for (i = 0; i < N_IN_URB; i++) {
+               buffer = (u8 *)__get_free_page(GFP_KERNEL);
+               if (!buffer)
+                       goto bail_out_error;
+               portdata->in_buffer[i] = buffer;
+
+               urb = usb_wwan_setup_urb(port, port->bulk_in_endpointAddress,
+                                               USB_DIR_IN, port,
+                                               buffer, IN_BUFLEN,
+                                               usb_wwan_indat_callback);
+               portdata->in_urbs[i] = urb;
+       }
+       for (i = 0; i < N_OUT_URB; i++) {
+               if (port->bulk_out_endpointAddress == -1)
+                       continue;
 
-               for (j = 0; j < N_IN_URB; j++) {
-                       buffer = (u8 *) __get_free_page(GFP_KERNEL);
-                       if (!buffer)
-                               goto bail_out_error;
-                       portdata->in_buffer[j] = buffer;
-               }
+               buffer = kmalloc(OUT_BUFLEN, GFP_KERNEL);
+               if (!buffer)
+                       goto bail_out_error2;
+               portdata->out_buffer[i] = buffer;
 
-               for (j = 0; j < N_OUT_URB; j++) {
-                       buffer = kmalloc(OUT_BUFLEN, GFP_KERNEL);
-                       if (!buffer)
-                               goto bail_out_error2;
-                       portdata->out_buffer[j] = buffer;
-               }
+               urb = usb_wwan_setup_urb(port, port->bulk_out_endpointAddress,
+                                               USB_DIR_OUT, port,
+                                               buffer, OUT_BUFLEN,
+                                               usb_wwan_outdat_callback);
+               portdata->out_urbs[i] = urb;
+       }
 
-               usb_set_serial_port_data(port, portdata);
+       usb_set_serial_port_data(port, portdata);
 
-               if (!port->interrupt_in_urb)
-                       continue;
+       if (port->interrupt_in_urb) {
                err = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
                if (err)
-                       dbg("%s: submit irq_in urb failed %d", __func__, err);
+                       dev_dbg(&port->dev, "%s: submit irq_in urb failed %d\n",
+                               __func__, err);
        }
-       usb_wwan_setup_urbs(serial);
+
        return 0;
 
 bail_out_error2:
-       for (j = 0; j < N_OUT_URB; j++)
-               kfree(portdata->out_buffer[j]);
+       for (i = 0; i < N_OUT_URB; i++) {
+               usb_free_urb(portdata->out_urbs[i]);
+               kfree(portdata->out_buffer[i]);
+       }
 bail_out_error:
-       for (j = 0; j < N_IN_URB; j++)
-               if (portdata->in_buffer[j])
-                       free_page((unsigned long)portdata->in_buffer[j]);
+       for (i = 0; i < N_IN_URB; i++) {
+               usb_free_urb(portdata->in_urbs[i]);
+               free_page((unsigned long)portdata->in_buffer[i]);
+       }
        kfree(portdata);
-       return 1;
+
+       return -ENOMEM;
 }
-EXPORT_SYMBOL(usb_wwan_startup);
+EXPORT_SYMBOL_GPL(usb_wwan_port_probe);
 
 int usb_wwan_port_remove(struct usb_serial_port *port)
 {