USB: mos7840: fix port-data memory leak
authorJohan Hovold <jhovold@gmail.com>
Thu, 25 Oct 2012 16:56:34 +0000 (18:56 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 25 Oct 2012 18:11:19 +0000 (11:11 -0700)
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 the indentation was kept intact using a do-while(0) in order
to facilitate review. A follow-up patch will remove it.

Compile-only tested.

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

index 84f8c106e5e904ed9d379b25339879336f753c27..bc3df86134feb4b1e54478d39dfd1518ff8bf2d9 100644 (file)
@@ -2327,49 +2327,45 @@ static int mos7840_calc_num_ports(struct usb_serial *serial)
        return mos7840_num_ports;
 }
 
-/****************************************************************************
- * mos7840_startup
- ****************************************************************************/
-
-static int mos7840_startup(struct usb_serial *serial)
+static int mos7840_port_probe(struct usb_serial_port *port)
 {
+       struct usb_serial *serial = port->serial;
        struct moschip_port *mos7840_port;
-       struct usb_device *dev;
-       int i, status;
+       int status;
+       int pnum;
        __u16 Data;
 
-       dev = serial->dev;
-
        /* we set up the pointers to the endpoints in the mos7840_open *
         * function, as the structures aren't created yet.             */
 
-       /* set up port private structures */
-       for (i = 0; i < serial->num_ports; ++i) {
-               dev_dbg(&dev->dev, "mos7840_startup: configuring port %d............\n", i);
+       pnum = port->number - serial->minor;
+
+       /* FIXME: remove do-while(0) loop used to keep stable patch minimal.
+        */
+       do {
+               dev_dbg(&port->dev, "mos7840_startup: configuring port %d............\n", pnum);
                mos7840_port = kzalloc(sizeof(struct moschip_port), GFP_KERNEL);
                if (mos7840_port == NULL) {
-                       dev_err(&dev->dev, "%s - Out of memory\n", __func__);
-                       status = -ENOMEM;
-                       i--; /* don't follow NULL pointer cleaning up */
-                       goto error;
+                       dev_err(&port->dev, "%s - Out of memory\n", __func__);
+                       return -ENOMEM;
                }
 
                /* Initialize all port interrupt end point to port 0 int
                 * endpoint. Our device has only one interrupt end point
                 * common to all port */
 
-               mos7840_port->port = serial->port[i];
-               mos7840_set_port_private(serial->port[i], mos7840_port);
+               mos7840_port->port = port;
+               mos7840_set_port_private(port, mos7840_port);
                spin_lock_init(&mos7840_port->pool_lock);
 
                /* minor is not initialised until later by
                 * usb-serial.c:get_free_serial() and cannot therefore be used
                 * to index device instances */
-               mos7840_port->port_num = i + 1;
-               dev_dbg(&dev->dev, "serial->port[i]->number = %d\n", serial->port[i]->number);
-               dev_dbg(&dev->dev, "serial->port[i]->serial->minor = %d\n", serial->port[i]->serial->minor);
-               dev_dbg(&dev->dev, "mos7840_port->port_num = %d\n", mos7840_port->port_num);
-               dev_dbg(&dev->dev, "serial->minor = %d\n", serial->minor);
+               mos7840_port->port_num = pnum + 1;
+               dev_dbg(&port->dev, "port->number = %d\n", port->number);
+               dev_dbg(&port->dev, "port->serial->minor = %d\n", port->serial->minor);
+               dev_dbg(&port->dev, "mos7840_port->port_num = %d\n", mos7840_port->port_num);
+               dev_dbg(&port->dev, "serial->minor = %d\n", serial->minor);
 
                if (mos7840_port->port_num == 1) {
                        mos7840_port->SpRegOffset = 0x0;
@@ -2396,115 +2392,115 @@ static int mos7840_startup(struct usb_serial *serial)
                        mos7840_port->ControlRegOffset = 0xd;
                        mos7840_port->DcrRegOffset = 0x1c;
                }
-               mos7840_dump_serial_port(serial->port[i], mos7840_port);
-               mos7840_set_port_private(serial->port[i], mos7840_port);
+               mos7840_dump_serial_port(port, mos7840_port);
+               mos7840_set_port_private(port, mos7840_port);
 
                /* enable rx_disable bit in control register */
-               status = mos7840_get_reg_sync(serial->port[i],
+               status = mos7840_get_reg_sync(port,
                                 mos7840_port->ControlRegOffset, &Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Reading ControlReg failed status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Reading ControlReg failed status-0x%x\n", status);
                        break;
                } else
-                       dev_dbg(&dev->dev, "ControlReg Reading success val is %x, status%d\n", Data, status);
+                       dev_dbg(&port->dev, "ControlReg Reading success val is %x, status%d\n", Data, status);
                Data |= 0x08;   /* setting driver done bit */
                Data |= 0x04;   /* sp1_bit to have cts change reflect in
                                   modem status reg */
 
                /* Data |= 0x20; //rx_disable bit */
-               status = mos7840_set_reg_sync(serial->port[i],
+               status = mos7840_set_reg_sync(port,
                                         mos7840_port->ControlRegOffset, Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Writing ControlReg failed(rx_disable) status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Writing ControlReg failed(rx_disable) status-0x%x\n", status);
                        break;
                } else
-                       dev_dbg(&dev->dev, "ControlReg Writing success(rx_disable) status%d\n", status);
+                       dev_dbg(&port->dev, "ControlReg Writing success(rx_disable) status%d\n", status);
 
                /* Write default values in DCR (i.e 0x01 in DCR0, 0x05 in DCR2
                   and 0x24 in DCR3 */
                Data = 0x01;
-               status = mos7840_set_reg_sync(serial->port[i],
+               status = mos7840_set_reg_sync(port,
                         (__u16) (mos7840_port->DcrRegOffset + 0), Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Writing DCR0 failed status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Writing DCR0 failed status-0x%x\n", status);
                        break;
                } else
-                       dev_dbg(&dev->dev, "DCR0 Writing success status%d\n", status);
+                       dev_dbg(&port->dev, "DCR0 Writing success status%d\n", status);
 
                Data = 0x05;
-               status = mos7840_set_reg_sync(serial->port[i],
+               status = mos7840_set_reg_sync(port,
                         (__u16) (mos7840_port->DcrRegOffset + 1), Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Writing DCR1 failed status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Writing DCR1 failed status-0x%x\n", status);
                        break;
                } else
-                       dev_dbg(&dev->dev, "DCR1 Writing success status%d\n", status);
+                       dev_dbg(&port->dev, "DCR1 Writing success status%d\n", status);
 
                Data = 0x24;
-               status = mos7840_set_reg_sync(serial->port[i],
+               status = mos7840_set_reg_sync(port,
                         (__u16) (mos7840_port->DcrRegOffset + 2), Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Writing DCR2 failed status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Writing DCR2 failed status-0x%x\n", status);
                        break;
                } else
-                       dev_dbg(&dev->dev, "DCR2 Writing success status%d\n", status);
+                       dev_dbg(&port->dev, "DCR2 Writing success status%d\n", status);
 
                /* write values in clkstart0x0 and clkmulti 0x20 */
                Data = 0x0;
-               status = mos7840_set_reg_sync(serial->port[i],
+               status = mos7840_set_reg_sync(port,
                                         CLK_START_VALUE_REGISTER, Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Writing CLK_START_VALUE_REGISTER failed status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Writing CLK_START_VALUE_REGISTER failed status-0x%x\n", status);
                        break;
                } else
-                       dev_dbg(&dev->dev, "CLK_START_VALUE_REGISTER Writing success status%d\n", status);
+                       dev_dbg(&port->dev, "CLK_START_VALUE_REGISTER Writing success status%d\n", status);
 
                Data = 0x20;
-               status = mos7840_set_reg_sync(serial->port[i],
+               status = mos7840_set_reg_sync(port,
                                        CLK_MULTI_REGISTER, Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Writing CLK_MULTI_REGISTER failed status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Writing CLK_MULTI_REGISTER failed status-0x%x\n", status);
                        goto error;
                } else
-                       dev_dbg(&dev->dev, "CLK_MULTI_REGISTER Writing success status%d\n", status);
+                       dev_dbg(&port->dev, "CLK_MULTI_REGISTER Writing success status%d\n", status);
 
                /* write value 0x0 to scratchpad register */
                Data = 0x00;
-               status = mos7840_set_uart_reg(serial->port[i],
+               status = mos7840_set_uart_reg(port,
                                                SCRATCH_PAD_REGISTER, Data);
                if (status < 0) {
-                       dev_dbg(&dev->dev, "Writing SCRATCH_PAD_REGISTER failed status-0x%x\n", status);
+                       dev_dbg(&port->dev, "Writing SCRATCH_PAD_REGISTER failed status-0x%x\n", status);
                        break;
                } else
-                       dev_dbg(&dev->dev, "SCRATCH_PAD_REGISTER Writing success status%d\n", status);
+                       dev_dbg(&port->dev, "SCRATCH_PAD_REGISTER Writing success status%d\n", status);
 
                /* Zero Length flag register */
                if ((mos7840_port->port_num != 1)
                    && (serial->num_ports == 2)) {
 
                        Data = 0xff;
-                       status = mos7840_set_reg_sync(serial->port[i],
+                       status = mos7840_set_reg_sync(port,
                                      (__u16) (ZLP_REG1 +
                                      ((__u16)mos7840_port->port_num)), Data);
-                       dev_dbg(&dev->dev, "ZLIP offset %x\n",
+                       dev_dbg(&port->dev, "ZLIP offset %x\n",
                                (__u16)(ZLP_REG1 + ((__u16) mos7840_port->port_num)));
                        if (status < 0) {
-                               dev_dbg(&dev->dev, "Writing ZLP_REG%d failed status-0x%x\n", i + 2, status);
+                               dev_dbg(&port->dev, "Writing ZLP_REG%d failed status-0x%x\n", pnum + 2, status);
                                break;
                        } else
-                               dev_dbg(&dev->dev, "ZLP_REG%d Writing success status%d\n", i + 2, status);
+                               dev_dbg(&port->dev, "ZLP_REG%d Writing success status%d\n", pnum + 2, status);
                } else {
                        Data = 0xff;
-                       status = mos7840_set_reg_sync(serial->port[i],
+                       status = mos7840_set_reg_sync(port,
                              (__u16) (ZLP_REG1 +
                              ((__u16)mos7840_port->port_num) - 0x1), Data);
-                       dev_dbg(&dev->dev, "ZLIP offset %x\n",
+                       dev_dbg(&port->dev, "ZLIP offset %x\n",
                                (__u16)(ZLP_REG1 + ((__u16) mos7840_port->port_num) - 0x1));
                        if (status < 0) {
-                               dev_dbg(&dev->dev, "Writing ZLP_REG%d failed status-0x%x\n", i + 1, status);
+                               dev_dbg(&port->dev, "Writing ZLP_REG%d failed status-0x%x\n", pnum + 1, status);
                                break;
                        } else
-                               dev_dbg(&dev->dev, "ZLP_REG%d Writing success status%d\n", i + 1, status);
+                               dev_dbg(&port->dev, "ZLP_REG%d Writing success status%d\n", pnum + 1, status);
 
                }
                mos7840_port->control_urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -2541,92 +2537,56 @@ static int mos7840_startup(struct usb_serial *serial)
                        mos7840_port->led_flag = false;
 
                        /* Turn off LED */
-                       mos7840_set_led_sync(serial->port[i],
+                       mos7840_set_led_sync(port,
                                                MODEM_CONTROL_REGISTER, 0x0300);
                }
-       }
+       } while (0);
 
-       /* Zero Length flag enable */
-       Data = 0x0f;
-       status = mos7840_set_reg_sync(serial->port[0], ZLP_REG5, Data);
-       if (status < 0) {
-               dev_dbg(&dev->dev, "Writing ZLP_REG5 failed status-0x%x\n", status);
-               goto error;
-       } else
-               dev_dbg(&dev->dev, "ZLP_REG5 Writing success status%d\n", status);
-
-       /* setting configuration feature to one */
-       usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
-                       (__u8) 0x03, 0x00, 0x01, 0x00, NULL, 0x00, MOS_WDR_TIMEOUT);
+       if (pnum == serial->num_ports - 1) {
+               /* Zero Length flag enable */
+               Data = 0x0f;
+               status = mos7840_set_reg_sync(serial->port[0], ZLP_REG5, Data);
+               if (status < 0) {
+                       dev_dbg(&port->dev, "Writing ZLP_REG5 failed status-0x%x\n", status);
+                       goto error;
+               } else
+                       dev_dbg(&port->dev, "ZLP_REG5 Writing success status%d\n", status);
+
+               /* setting configuration feature to one */
+               usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
+                               0x03, 0x00, 0x01, 0x00, NULL, 0x00,
+                               MOS_WDR_TIMEOUT);
+       }
        return 0;
 error:
-       for (/* nothing */; i >= 0; i--) {
-               mos7840_port = mos7840_get_port_private(serial->port[i]);
+       kfree(mos7840_port->dr);
+       kfree(mos7840_port->ctrl_buf);
+       usb_free_urb(mos7840_port->control_urb);
+       kfree(mos7840_port);
 
-               kfree(mos7840_port->dr);
-               kfree(mos7840_port->ctrl_buf);
-               usb_free_urb(mos7840_port->control_urb);
-               kfree(mos7840_port);
-       }
        return status;
 }
 
-/****************************************************************************
- * mos7840_disconnect
- *     This function is called whenever the device is removed from the usb bus.
- ****************************************************************************/
-
-static void mos7840_disconnect(struct usb_serial *serial)
+static int mos7840_port_remove(struct usb_serial_port *port)
 {
-       int i;
-       unsigned long flags;
        struct moschip_port *mos7840_port;
 
-       /* check for the ports to be closed,close the ports and disconnect */
+       mos7840_port = mos7840_get_port_private(port);
 
-       /* free private structure allocated for serial port  *
-        * stop reads and writes on all ports                */
+       if (mos7840_port->has_led) {
+               /* Turn off LED */
+               mos7840_set_led_sync(port, MODEM_CONTROL_REGISTER, 0x0300);
 
-       for (i = 0; i < serial->num_ports; ++i) {
-               mos7840_port = mos7840_get_port_private(serial->port[i]);
-               if (mos7840_port) {
-                       usb_kill_urb(mos7840_port->control_urb);
-               }
+               del_timer_sync(&mos7840_port->led_timer1);
+               del_timer_sync(&mos7840_port->led_timer2);
        }
-}
-
-/****************************************************************************
- * mos7840_release
- *     This function is called when the usb_serial structure is freed.
- ****************************************************************************/
-
-static void mos7840_release(struct usb_serial *serial)
-{
-       int i;
-       struct moschip_port *mos7840_port;
+       usb_kill_urb(mos7840_port->control_urb);
+       usb_free_urb(mos7840_port->control_urb);
+       kfree(mos7840_port->ctrl_buf);
+       kfree(mos7840_port->dr);
+       kfree(mos7840_port);
 
-       /* check for the ports to be closed,close the ports and disconnect */
-
-       /* free private structure allocated for serial port  *
-        * stop reads and writes on all ports                */
-
-       for (i = 0; i < serial->num_ports; ++i) {
-               mos7840_port = mos7840_get_port_private(serial->port[i]);
-               if (mos7840_port) {
-                       if (mos7840_port->has_led) {
-                               /* Turn off LED */
-                               mos7840_set_led_sync(mos7840_port->port,
-                                               MODEM_CONTROL_REGISTER, 0x0300);
-
-                               del_timer_sync(&mos7840_port->led_timer1);
-                               del_timer_sync(&mos7840_port->led_timer2);
-                       }
-                       usb_free_urb(mos7840_port->control_urb);
-                       kfree(mos7840_port->ctrl_buf);
-                       kfree(mos7840_port->dr);
-                       kfree(mos7840_port);
-               }
-       }
+       return 0;
 }
 
 static struct usb_serial_driver moschip7840_4port_device = {
@@ -2654,9 +2614,8 @@ static struct usb_serial_driver moschip7840_4port_device = {
        .tiocmget = mos7840_tiocmget,
        .tiocmset = mos7840_tiocmset,
        .get_icount = mos7840_get_icount,
-       .attach = mos7840_startup,
-       .disconnect = mos7840_disconnect,
-       .release = mos7840_release,
+       .port_probe = mos7840_port_probe,
+       .port_remove = mos7840_port_remove,
        .read_bulk_callback = mos7840_bulk_in_callback,
        .read_int_callback = mos7840_interrupt_callback,
 };