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

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>

1 files changed, 50 insertions, 74 deletions

diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index e42aa398ed3..61a73ad1a18 100644
--- 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)
@@ -472,101 +474,75 @@ 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);
+	portdata = kzalloc(sizeof(*portdata), GFP_KERNEL);
+	if (!portdata)
+		return -ENOMEM;
 
-		/* 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);
-		}
+	init_usb_anchor(&portdata->delayed);
 
-		/* 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);
-		}
+	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;
 	}
-}
-
-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;
 
-	/* 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) {
-			dev_dbg(&port->dev, "%s: kmalloc for usb_wwan_port_private (%d) failed!.\n",
-				__func__, i);
-			return 1;
-		}
-		init_usb_anchor(&portdata->delayed);
+	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)
 			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)
 {