Re: [PATCH] staging: rtl8712: fix potential leak in r871x_wx_set_enc_ext()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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@gmx.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@linux.intel.com>
Intel Finland Oy