staging: rtl8712: fix potential leak in r871x_wx_set_enc_ext()

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.

Signed-off-by Christian Engelmayer <[email protected]>

---
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(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +0 -7

index 23d539d..1eca992 100644
@@ -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";

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +6 -0

+	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;

drivers/staging/rtl8712/rtl871x_ioctl_linux.c static int r871x_wx_set_enc_ext(struct net_device *dev, · line 1822

Quoted 7 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +3 -0

@@ -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?

Quoted 11 lines · Andy Shevchenko

drivers/staging/rtl8712/rtl871x_ioctl_linux.c static int r871x_wx_set_enc_ext(struct net_device *dev, · line 1822

Quoted 7 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +3 -0

@@ -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?

Normally, that's the kind of thing we would do in a separate patch because or the "one thing per patch" rule. Eventually someone will do a driver wide replacement of _malloc(). Or if the bug fixer wanted, he could do it in this patch because it is on the same line and all so it counts as a "minor closely related change." Either way is fine.

Another way to say this is that since the _malloc() was there in the original code and Christian didn't introduce it, then we shouldn't reject his patch because of it. This is staging code, and there are so many problems that if you start trying to fix everything you'll just get lost.

In my experience v2 patches are much harder to write than v1 patches. Twice in the past few days I have messed up the subject line in my v2 patches.