Re: [PATCH] serial: vt8500: fix possible memory leak in vt8500_serial_probe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2012-10-08 at 08:47 +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@xxxxxxxxxxxxxxxxx>
> 
> 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 <yongjun_wei@xxxxxxxxxxxxxxxxx>
> ---
>  drivers/tty/serial/vt8500_serial.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
> index 205d4cf..43fc369 100644
> --- a/drivers/tty/serial/vt8500_serial.c
> +++ b/drivers/tty/serial/vt8500_serial.c
> @@ -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;
> 

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 <linux@xxxxxxxxxxxxxxx>


Regards
Tony P

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


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux