Skip to contentSkip to conversation

serial: vt8500: fix possible memory leak in vt8500_serial_probe()

PATCH Superseded Wei Yongjun 2 participants 2 messages Oct 8, 2012

Subsystems (2)
TTY layer and serial drivers Arm/VT8500 Arm architecture

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Wei Yongjun

author
#0 lore raw

PATCH 1/1 serial: vt8500: fix possible memory leak in vt8500_serial_probe()

From: Wei Yongjun <[email protected]>

vt8500_port is malloced in vt8500_serial_probe() and should be freed before leaving from the error handling cases, otherwise it will cause memory leak.

dpatch engine is used to auto generate this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by Wei Yongjun <[email protected]>
---
 drivers/tty/serial/vt8500_serial.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/tty/serial/vt8500_serial.c +6 -3
index 205d4cf..43fc369 100644
@@ -584,13 +584,16 @@ static int __devinit vt8500_serial_probe(struct platform_device *pdev)
 					sizeof(vt8500_ports_in_use));
 	}
 
-	if (port > VT8500_MAX_PORTS)
-		return -ENODEV;
+	if (port > VT8500_MAX_PORTS) {
+		ret = -ENODEV;
+		goto err;
+	}
 
 	/* reserve the port id */
 	if (test_and_set_bit(port, &vt8500_ports_in_use)) {
 		/* port already in use - shouldn't really happen */
-		return -EBUSY;
+		ret = -EBUSY;
+		goto err;
 	}
 
 	vt8500_port->uart.type = PORT_VT8500;
#1 lore raw

PATCH 1/1 serial: vt8500: fix possible memory leak in vt8500_serial_probe()

Quoted 29 lines · Wei Yongjun

From: Wei Yongjun <[email protected]>

vt8500_port is malloced in vt8500_serial_probe() and should be freed before leaving from the error handling cases, otherwise it will cause memory leak.

dpatch engine is used to auto generate this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by Wei Yongjun <[email protected]>
---
 drivers/tty/serial/vt8500_serial.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/tty/serial/vt8500_serial.c +4 -2
index 205d4cf..43fc369 100644
@@ -584,13 +584,16 @@ static int __devinit vt8500_serial_probe(struct platform_device *pdev)
 					sizeof(vt8500_ports_in_use));
 	}
 
-	if (port > VT8500_MAX_PORTS)
-		return -ENODEV;
+	if (port > VT8500_MAX_PORTS) {
+		ret = -ENODEV;
+		goto err;
+	}

drivers/tty/serial/vt8500_serial.c static int __devinit vt8500_serial_probe(struct platform_device *pdev) · line 599

Quoted 10 lines
drivers/tty/serial/vt8500_serial.c +2 -1
 
 	/* reserve the port id */
 	if (test_and_set_bit(port, &vt8500_ports_in_use)) {
 		/* port already in use - shouldn't really happen */
-		return -EBUSY;
+		ret = -EBUSY;
+		goto err;
 	}
 
 	vt8500_port->uart.type = PORT_VT8500;

You are correct - this is an error but wouldn't it be better to simply move the allocation of vt8500_port after these test since it's not used until later.

It would save two possible instances of having to alloc/release a block of memory that is never used and possibly create unnecessary memory fragmentation.

Either way, this patch fixes the problem, so you can add my Ack'd if my suggestion is considered nit-picky.

Acked-by Tony Prisk <[email protected]>

Regards
Tony P