Skip to contentSkip to conversation

staging: rtl8723au: fix potential leak in update_bcn_wps_ie()

PATCH In review Christian Engelmayer 3 participants 4 messages May 1, 2014 Staging

lore
Share & tools
lore

Christian Engelmayer

author
#0 lore raw

PATCH 1/1 staging: rtl8723au: fix potential leak in update_bcn_wps_ie()

Fix a potential leak in the error path of function update_bcn_wps_ie(). Make sure that allocated memory for 'pbackup_remainder_ie' is freed upon return. Detected by Coverity - CID 1077718.

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/rtl8723au/core/rtw_ap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/rtl8723au/core/rtw_ap.c +3 -3
index 9b31412..f2c78a7 100644
@@ -1276,7 +1276,7 @@ static void update_bcn_wps_ie(struct rtw_adapter *padapter)
 
 	pwps_ie_src = pmlmepriv->wps_beacon_ie;
 	if (pwps_ie_src == NULL)
-		return;
+		goto exit;
 
 	wps_ielen = (uint)pwps_ie_src[1];/* to get ie data len */
 	if ((wps_offset+wps_ielen+2+remainder_ielen)<= MAX_IE_SZ)

@@ -1291,8 +1291,8 @@ static void update_bcn_wps_ie(struct rtw_adapter *padapter)
 		pnetwork->IELength = wps_offset + (wps_ielen+2) + remainder_ielen;
 	}
 
-	if (pbackup_remainder_ie)
-		kfree(pbackup_remainder_ie);
+exit:
+	kfree(pbackup_remainder_ie);
 }
 
 static void update_bcn_p2p_ie(struct rtw_adapter *padapter)
Signature · 1 line
1.9.1
#1 lore raw
Quoted 4 lines · Christian Engelmayer

Fix a potential leak in the error path of function update_bcn_wps_ie(). Make sure that allocated memory for 'pbackup_remainder_ie' is freed upon return. Detected by Coverity - CID 1077718.

        if (remainder_ielen > 0) {
                pbackup_remainder_ie = kmalloc(remainder_ielen, GFP_ATOMIC);
                if (pbackup_remainder_ie)
                        memcpy(pbackup_remainder_ie, premainder_ie,
                               remainder_ielen);
        }

        pwps_ie_src = pmlmepriv->wps_beacon_ie;
        if (pwps_ie_src == NULL)
                return;

Maybe just check pwps_ie_src earlier?

Signature · 1 line
Mateusz Guzik
author
#2 lore raw
Quoted 18 lines · Mateusz Guzik
Quoted 4 lines · Christian Engelmayer

Fix a potential leak in the error path of function update_bcn_wps_ie(). Make sure that allocated memory for 'pbackup_remainder_ie' is freed upon return. Detected by Coverity - CID 1077718.

        if (remainder_ielen > 0) {
                pbackup_remainder_ie = kmalloc(remainder_ielen, GFP_ATOMIC);
                if (pbackup_remainder_ie)
                        memcpy(pbackup_remainder_ie, premainder_ie,
                               remainder_ielen);
        }

        pwps_ie_src = pmlmepriv->wps_beacon_ie;
        if (pwps_ie_src == NULL)
                return;
Quoted 2 lines

Maybe just check pwps_ie_src earlier?

You are right, I see no reason why this cannot be done early in the function.

drivers/staging/rtl8723au/core/rtw_ap.c +4 -4
index 9b31412..da028c535 100644
@@ -1256,6 +1256,10 @@ static void update_bcn_wps_ie(struct rtw_adapter *padapter)
 
 	DBG_8723A("%s\n", __func__);
 
+	pwps_ie_src = pmlmepriv->wps_beacon_ie;
+	if (pwps_ie_src == NULL)
+		return;
+
 	pwps_ie = rtw_get_wps_ie23a(ie+_FIXED_IE_LENGTH_, ielen-_FIXED_IE_LENGTH_, NULL, &wps_ielen);
 
 	if (pwps_ie == NULL || wps_ielen == 0)

@@ -1274,10 +1278,6 @@ static void update_bcn_wps_ie(struct rtw_adapter *padapter)
 			       remainder_ielen);
 	}
 
-	pwps_ie_src = pmlmepriv->wps_beacon_ie;
-	if (pwps_ie_src == NULL)
-		return;
-
 	wps_ielen = (uint)pwps_ie_src[1];/* to get ie data len */
 	if ((wps_offset+wps_ielen+2+remainder_ielen)<= MAX_IE_SZ)
 	{

Regards,
Christian

#3 lore raw
Quoted 19 lines · Christian Engelmayer
Quoted 18 lines · Mateusz Guzik
Quoted 4 lines · Christian Engelmayer

Fix a potential leak in the error path of function update_bcn_wps_ie(). Make sure that allocated memory for 'pbackup_remainder_ie' is freed upon return. Detected by Coverity - CID 1077718.

        if (remainder_ielen > 0) {
                pbackup_remainder_ie = kmalloc(remainder_ielen, GFP_ATOMIC);
                if (pbackup_remainder_ie)
                        memcpy(pbackup_remainder_ie, premainder_ie,
                               remainder_ielen);
        }

        pwps_ie_src = pmlmepriv->wps_beacon_ie;
        if (pwps_ie_src == NULL)
                return;
Quoted 4 lines
Quoted 2 lines

Maybe just check pwps_ie_src earlier?

You are right, I see no reason why this cannot be done early in the function.

Looks good to me - if you send me a patch with a commit message and a Signed-off-by, I'll add it to the rtl8723au driver tree and push it to Greg with my next set of changes.

Cheers,
Jes