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

Andy Shevchenko
2014-05-07T06:55:52Z

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