linux-kernel - Re: [PATCH] staging: rtl8188eu: fix potential leak in rtw_set

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: <20140501090924.GK26890@mwanda>
Date:	Thu, 1 May 2014 12:09:24 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Christian Engelmayer <cengelma@....at>
Cc:	devel@...verdev.osuosl.org, mcgrof@...not-panic.com,
	gregkh@...uxfoundation.org, oat.elena@...il.com,
	linux-kernel@...r.kernel.org, Larry.Finger@...inger.net
Subject: Re: [PATCH] staging: rtl8188eu: fix potential leak in rtw_set_key()

On Thu, May 01, 2014 at 10:38:11AM +0200, Christian Engelmayer wrote:
> Fix a potential leak in the error path of rtw_set_key(). In case the requested
> algorithm is not supported by the driver, the function returns without
> enqueuing or freeing the already allocated command and parameter structs. Use
> a centralized exit path and make sure that all memory is freed correctly.
> Detected by Coverity - CID 1077716, 1077717.
> 
> Signed-off-by: Christian Engelmayer <cengelma@....at>
> ---
> Compile tested and applies against branch staging-next of tree
> git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> ---
>  drivers/staging/rtl8188eu/core/rtw_mlme.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 769d4dd..275ae7b 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -1729,13 +1729,12 @@ int rtw_set_key(struct adapter *adapter, struct security_priv *psecuritypriv, in
>  	pcmd = (struct	cmd_obj *)rtw_zmalloc(sizeof(struct	cmd_obj));
>  	if (pcmd == NULL) {
>  		res = _FAIL;  /* try again */
> -		goto exit;
> +		goto fail_noobj;

Just return here.  The do nothing goto is misleading because you assume
it is a there for a purpose but then when you read all the way to the
end, it doesn't do anything useful.  It doesn't do anything at all.

The label statements should be based on the thing which is labeled and
not the goto locations.  In this case it should be something like
"err_free_cmd" and "err_free_keyparm".  The "fail" label is too generic
so it's not a good name.

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/