linux-kernel - Re: [PATCH v2 4/5] staging: rtl8188eu: fix potential leak in rtw_mp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20140507200806.GW4963@mwanda>
Date:	Wed, 7 May 2014 23:08:06 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Christian Engelmayer <cengelma@....at>
Cc:	devel@...verdev.osuosl.org, w-lkml@...enslange-mailadresse.de,
	gregkh@...uxfoundation.org, peter.p.waskiewicz.jr@...el.com,
	oat.elena@...il.com, linux-kernel@...r.kernel.org,
	teobaluta@...il.com, Larry.Finger@...inger.net
Subject: Re: [PATCH v2 4/5] staging: rtl8188eu: fix potential leak in
 rtw_mp_SetRFPath()

On Wed, May 07, 2014 at 09:33:20PM +0200, Christian Engelmayer wrote:
> Function rtw_mp_SetRFPath() dynamically allocates a temporary buffer that
> is not freed in all error paths. Use a centralized exit path and make sure
> that all memory is freed correctly. Detected by Coverity - CID 1077714.
> 
> Signed-off-by: Christian Engelmayer <cengelma@....at>
> ---
>  drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> index 8b1579b..ea5e1f8 100644
> --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> @@ -7321,11 +7321,14 @@ static int rtw_mp_SetRFPath(struct net_device *dev,
>  	struct adapter *padapter = rtw_netdev_priv(dev);
>  	char	*input = kmalloc(wrqu->data.length, GFP_KERNEL);

You didn't introduce this so there is no need to resend but doing the
allocation in the declaration block is nasty style.  No one reads the
declaration block and it is a common cause of bugs.  For example, the
bug you are fixing here.  ;)

>  	u8 bMain = 1, bTurnoff = 1;
> +	int ret = 0;

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/