linux-kernel - Re: [PATCH] staging: rtl8712: fix potential leak in r871x_wx_set_enc

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 07 May 2014 09:55:47 +0300
From:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:	Christian Engelmayer <cengelma@....at>
Cc:	devel@...verdev.osuosl.org, Larry.Finger@...inger.net,
	florian.c.schilhabel@...glemail.com, gregkh@...uxfoundation.org,
	linuxgeek@...il.com, paul.gortmaker@...driver.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8712: fix potential leak in
 r871x_wx_set_enc_ext()

On Thu, 2014-05-01 at 11:45 +0200, Christian Engelmayer wrote:
> Fix a potential leak in the error path of r871x_wx_set_enc_ext(). In case the
> requested algorithm is not supported by the driver, the function returns
> without freeing the already allocated 'param' struct. Move the input
> verification to the beginning of the function so that the direct return is
> safe. Detected by Coverity - CID 144373.
> 


One comment below.


> 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/rtl8712/rtl871x_ioctl_linux.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 23d539d..1eca992 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -1801,13 +1801,6 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
>  	u32 param_len;
>  	int ret = 0;
>  
> -	param_len = sizeof(struct ieee_param) + pext->key_len;
> -	param = (struct ieee_param *)_malloc(param_len);
> -	if (param == NULL)
> -		return -ENOMEM;
> -	memset(param, 0, param_len);
> -	param->cmd = IEEE_CMD_SET_ENCRYPTION;
> -	memset(param->sta_addr, 0xff, ETH_ALEN);
>  	switch (pext->alg) {
>  	case IW_ENCODE_ALG_NONE:
>  		alg_name = "none";
> @@ -1824,6 +1817,15 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
>  	default:
>  		return -EINVAL;
>  	}
> +
> +	param_len = sizeof(struct ieee_param) + pext->key_len;
> +	param = (struct ieee_param *)_malloc(param_len);

While you are here could you substitute _malloc by kzalloc and remove
explicit casting and memset?

> +	if (param == NULL)
> +		return -ENOMEM;
> +	memset(param, 0, param_len);
> +	param->cmd = IEEE_CMD_SET_ENCRYPTION;
> +	memset(param->sta_addr, 0xff, ETH_ALEN);
> +
>  	strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
>  	if (pext->ext_flags & IW_ENCODE_EXT_GROUP_KEY)
>  		param->u.crypt.set_tx = 0;


-- 
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Intel Finland Oy

--
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/