[PATCH] staging: rtl8712: fix potential leaks in r8712_set_key()

[PATCH] staging: rtl8712: fix potential leaks in r8712_set_key()

[Christian Engelmayer]

[-- Attachment #1: Type: text/plain, Size: 2652 bytes --]

Fix potential leaks in the error paths of r8712_set_key(). In case the
algorithm specific checks fail, 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 144370, 144371.

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_mlme.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 3ea99ae..23fd8c1 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -1243,14 +1243,15 @@ sint r8712_set_key(struct _adapter *adapter,
 	struct cmd_obj *pcmd;
 	struct setkey_parm *psetkeyparm;
 	u8 keylen;
+	sint ret = _SUCCESS;
 
 	pcmd = (struct cmd_obj *)_malloc(sizeof(struct cmd_obj));
 	if (pcmd == NULL)
 		return _FAIL;
 	psetkeyparm = (struct setkey_parm *)_malloc(sizeof(struct setkey_parm));
 	if (psetkeyparm == NULL) {
-		kfree((unsigned char *)pcmd);
-		return _FAIL;
+		ret = _FAIL;
+		goto err_free_cmd;
 	}
 	memset(psetkeyparm, 0, sizeof(struct setkey_parm));
 	if (psecuritypriv->AuthAlgrthm == 2) { /* 802.1X */
@@ -1274,23 +1275,28 @@ sint r8712_set_key(struct _adapter *adapter,
 			psecuritypriv->DefKey[keyid].skey, keylen);
 		break;
 	case _TKIP_:
-		if (keyid < 1 || keyid > 2)
-			return _FAIL;
+		if (keyid < 1 || keyid > 2) {
+			ret = _FAIL;
+			goto err_free_parm;
+		}
 		keylen = 16;
 		memcpy(psetkeyparm->key,
 			&psecuritypriv->XGrpKey[keyid - 1], keylen);
 		psetkeyparm->grpkey = 1;
 		break;
 	case _AES_:
-		if (keyid < 1 || keyid > 2)
-			return _FAIL;
+		if (keyid < 1 || keyid > 2) {
+			ret = _FAIL;
+			goto err_free_parm;
+		}
 		keylen = 16;
 		memcpy(psetkeyparm->key,
 			&psecuritypriv->XGrpKey[keyid - 1], keylen);
 		psetkeyparm->grpkey = 1;
 		break;
 	default:
-		return _FAIL;
+		ret = _FAIL;
+		goto err_free_parm;
 	}
 	pcmd->cmdcode = _SetKey_CMD_;
 	pcmd->parmbuf = (u8 *)psetkeyparm;
@@ -1299,7 +1305,13 @@ sint r8712_set_key(struct _adapter *adapter,
 	pcmd->rspsz = 0;
 	_init_listhead(&pcmd->list);
 	r8712_enqueue_cmd(pcmdpriv, pcmd);
-	return _SUCCESS;
+	return ret;
+
+err_free_parm:
+	kfree(psetkeyparm);
+err_free_cmd:
+	kfree(pcmd);
+	return ret;
 }
 
 /* adjust IEs for r8712_joinbss_cmd in WMM */
-- 
1.9.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] staging: rtl8712: fix potential leaks in r8712_set_key()

[Greg KH]

On Thu, May 01, 2014 at 11:54:02PM +0200, Christian Engelmayer wrote:
> Fix potential leaks in the error paths of r8712_set_key(). In case the
> algorithm specific checks fail, 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 144370, 144371.
> 
> 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

This doesn't apply either, and neither does one of your other patches,
what is going on?

Can you just refresh all of the outstanding patches you have sent me
that I have not applied and resend them?

thanks,

greg k-h

Re: [PATCH] staging: rtl8712: fix potential leaks in r8712_set_key()

[Christian Engelmayer]

On Sat, 3 May 2014 20:27:35 -0400, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, May 01, 2014 at 11:54:02PM +0200, Christian Engelmayer wrote:
> > Fix potential leaks in the error paths of r8712_set_key(). In case the
> > algorithm specific checks fail, 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 144370, 144371.
> > 
> > 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
> 
> This doesn't apply either, and neither does one of your other patches,
> what is going on?

Greg, I am sorry that last weeks set of staging patches had problems. It
is not my intention to waste Your time.

I could reproduce the issue by running git am on my patches as received
back from the mailing list. Most of the set failed as the mails were split
incorrectly. I started rebasing the patches to the current staging-next
and fixed my mail agents settings to generate no multipart, 7bit text/plain
us-ascii. Thus the same set sent to myself applies now to a fresh clone of
Your tree.


git checkout -b integration-test origin/staging-next 
Branch integration-test set up to track remote branch staging-next from origin.
Switched to a new branch 'integration-test'

git am ./staging.mbox
Applying: staging: binder: fix usage of uninit scalar in binder_transaction()
Applying: staging: comedi: ii_pci20kc: fix usage of uninit scalar in ii20k_attach()
Applying: staging: rtl8188eu: fix potential leak in rtw_set_key()
Applying: staging: rtl8188eu: fix potential leak in rtw_wx_read32()
Applying: staging: rtl8188eu: fix potential leak in update_bcn_wps_ie()
Applying: staging: rtl8712: fix potential leaks in r8712_set_key()
Applying: staging: rtl8723au: Remove unused pointer in rtw_wdev_free()
Applying: staging: rtl8712: fix potential leak in r871x_wx_set_enc_ext()
Applying: staging: silicom: Remove needless calls of get_status_port_fn()
Applying: staging: silicom: Remove unused pointer in bypass_init_module()
Applying: staging: vt6656: fix potential leak in vt6656_hostap_ioctl()
Applying: staging: rtl8188eu: fix potential leak in rtw_wx_set_enc_ext()
Applying: staging: rtl8188eu: fix potential leak in rtw_mp_QueryDrv()
Applying: staging: rtl8188eu: fix potential leak in rtw_mp_SetRFPath()
Applying: staging: rtl8188eu: fix potential leak in rtw_mp_pwrtrk()


> Can you just refresh all of the outstanding patches you have sent me
> that I have not applied and resend them?

Of course. In case there are no objections or further hints, I will check
the refreshed patches later this week when I have got a bit more time and
will resend them.

Regards,
Christian