Fix port-data memory leak by moving port data allocation and
deallocation to port_probe and port_remove.

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

Note that this patch also fixes a second port-data memory leak in the
error path of attach should port data allocation fail for any port but
the first.

Note also that urb-count for multi-port interfaces has not been changed
even though the usb-serial port number is now determined from the port
and interface minor numbers.

Compile-only tested.

Cc: <sta...@vger.kernel.org>
Signed-off-by: Johan Hovold <jhov...@gmail.com>
---
 drivers/usb/serial/sierra.c | 112 ++++++++++++++++++++------------------------
 1 file changed, 51 insertions(+), 61 deletions(-)

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 01d882c..598e996 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -884,12 +884,6 @@ static void sierra_dtr_rts(struct usb_serial_port *port, 
int on)
 
 static int sierra_startup(struct usb_serial *serial)
 {
-       struct usb_serial_port *port;
-       struct sierra_port_private *portdata;
-       struct sierra_iface_info *himemoryp = NULL;
-       int i;
-       u8 ifnum;
-
        /* Set Device mode to D0 */
        sierra_set_power_state(serial->dev, 0x0000);
 
@@ -897,68 +891,63 @@ static int sierra_startup(struct usb_serial *serial)
        if (nmea)
                sierra_vsc_set_nmea(serial->dev, 1);
 
-       /* 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 "
-                               "sierra_port_private (%d) failed!\n",
-                               __func__, i);
-                       return -ENOMEM;
-               }
-               spin_lock_init(&portdata->lock);
-               init_usb_anchor(&portdata->active);
-               init_usb_anchor(&portdata->delayed);
-               ifnum = i;
-               /* Assume low memory requirements */
-               portdata->num_out_urbs = N_OUT_URB;
-               portdata->num_in_urbs  = N_IN_URB;
-
-               /* Determine actual memory requirements */
-               if (serial->num_ports == 1) {
-                       /* Get interface number for composite device */
-                       ifnum = sierra_calc_interface(serial);
-                       himemoryp =
-                           (struct sierra_iface_info *)&typeB_interface_list;
-                       if (is_himemory(ifnum, himemoryp)) {
-                               portdata->num_out_urbs = N_OUT_URB_HM;
-                               portdata->num_in_urbs  = N_IN_URB_HM;
-                       }
-               }
-               else {
-                       himemoryp =
-                           (struct sierra_iface_info *)&typeA_interface_list;
-                       if (is_himemory(i, himemoryp)) {
-                               portdata->num_out_urbs = N_OUT_URB_HM;
-                               portdata->num_in_urbs  = N_IN_URB_HM;
-                       }
-               }
-               dev_dbg(&serial->dev->dev,
-                       "Memory usage (urbs) interface #%d, in=%d, out=%d\n",
-                       ifnum,portdata->num_in_urbs, portdata->num_out_urbs );
-               /* Set the port private data pointer */
-               usb_set_serial_port_data(port, portdata);
+       return 0;
+}
+
+static int sierra_port_probe(struct usb_serial_port *port)
+{
+       struct usb_serial *serial = port->serial;
+       struct sierra_port_private *portdata;
+       const struct sierra_iface_info *himemoryp;
+       u8 ifnum;
+
+       portdata = kzalloc(sizeof(*portdata), GFP_KERNEL);
+       if (!portdata)
+               return -ENOMEM;
+
+       spin_lock_init(&portdata->lock);
+       init_usb_anchor(&portdata->active);
+       init_usb_anchor(&portdata->delayed);
+
+       /* Assume low memory requirements */
+       portdata->num_out_urbs = N_OUT_URB;
+       portdata->num_in_urbs  = N_IN_URB;
+
+       /* Determine actual memory requirements */
+       if (serial->num_ports == 1) {
+               /* Get interface number for composite device */
+               ifnum = sierra_calc_interface(serial);
+               himemoryp = &typeB_interface_list;
+       } else {
+               /* This is really the usb-serial port number of the interface
+                * rather than the interface number.
+                */
+               ifnum = port->number - serial->minor;
+               himemoryp = &typeA_interface_list;
        }
 
+       if (is_himemory(ifnum, himemoryp)) {
+               portdata->num_out_urbs = N_OUT_URB_HM;
+               portdata->num_in_urbs  = N_IN_URB_HM;
+       }
+
+       dev_dbg(&port->dev,
+                       "Memory usage (urbs) interface #%d, in=%d, out=%d\n",
+                       ifnum, portdata->num_in_urbs, portdata->num_out_urbs);
+
+       usb_set_serial_port_data(port, portdata);
+
        return 0;
 }
 
-static void sierra_release(struct usb_serial *serial)
+static int sierra_port_remove(struct usb_serial_port *port)
 {
-       int i;
-       struct usb_serial_port *port;
        struct sierra_port_private *portdata;
 
-       for (i = 0; i < serial->num_ports; ++i) {
-               port = serial->port[i];
-               if (!port)
-                       continue;
-               portdata = usb_get_serial_port_data(port);
-               if (!portdata)
-                       continue;
-               kfree(portdata);
-       }
+       portdata = usb_get_serial_port_data(port);
+       kfree(portdata);
+
+       return 0;
 }
 
 #ifdef CONFIG_PM
@@ -1061,7 +1050,8 @@ static struct usb_serial_driver sierra_device = {
        .tiocmget          = sierra_tiocmget,
        .tiocmset          = sierra_tiocmset,
        .attach            = sierra_startup,
-       .release           = sierra_release,
+       .port_probe        = sierra_port_probe,
+       .port_remove       = sierra_port_remove,
        .suspend           = sierra_suspend,
        .resume            = sierra_resume,
        .read_int_callback = sierra_instat_callback,
-- 
1.7.12.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to