linux-kernel - [PATCH 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tencent_2F72CC5068850B2BEDFC2B8058303FE6520A@qq.com>
Date:   Wed, 30 Mar 2022 23:29:22 +0800
From:   xkernel.wang@...mail.com
To:     Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        gregkh@...uxfoundation.org
Cc:     linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Xiaoke Wang <xkernel.wang@...mail.com>
Subject: [PATCH 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit_priv()

From: Xiaoke Wang <xkernel.wang@...mail.com>

In _rtw_init_xmit_priv(), there are several error paths for allocation
failures without releasing the resources.
To properly release them, there are several error lables and some
modifications for rtw_os_xmit_resource_free().
The `for(; i >= 0; i--)` is to only release the explored items.
While the modifications for rtw_os_xmit_resource_free() is 
corresponding to the logic of rtw_os_xmit_resource_alloc() to break 
unintentional wrong operations. 

Signed-off-by: Xiaoke Wang <xkernel.wang@...mail.com>
---
 drivers/staging/r8188eu/core/rtw_xmit.c     | 41 ++++++++++++++++++---
 drivers/staging/r8188eu/os_dep/xmit_linux.c |  8 +++-
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 5888979..813bddf 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -112,7 +112,7 @@ s32	_rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 
 	if (!pxmitpriv->pallocated_xmitbuf) {
 		res = _FAIL;
-		goto exit;
+		goto free_frame_buf;
 	}
 
 	pxmitpriv->pxmitbuf = (u8 *)N_BYTE_ALIGMENT((size_t)(pxmitpriv->pallocated_xmitbuf), 4);
@@ -134,7 +134,12 @@ s32	_rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 			msleep(10);
 			res = rtw_os_xmit_resource_alloc(padapter, pxmitbuf, (MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ));
 			if (res == _FAIL) {
-				goto exit;
+				pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
+				for (; i >= 0; i--) {
+					rtw_os_xmit_resource_free(padapter, pxmitbuf, (MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ));
+					pxmitbuf++;
+				}
+				goto free_xmitbuf;
 			}
 		}
 
@@ -153,7 +158,7 @@ s32	_rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 
 	if (!pxmitpriv->pallocated_xmit_extbuf) {
 		res = _FAIL;
-		goto exit;
+		goto free_pxmitbuf;
 	}
 
 	pxmitpriv->pxmit_extbuf = (u8 *)N_BYTE_ALIGMENT((size_t)(pxmitpriv->pallocated_xmit_extbuf), 4);
@@ -169,8 +174,12 @@ s32	_rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 
 		res = rtw_os_xmit_resource_alloc(padapter, pxmitbuf, max_xmit_extbuf_size + XMITBUF_ALIGN_SZ);
 		if (res == _FAIL) {
-			res = _FAIL;
-			goto exit;
+			pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmit_extbuf;
+			for (; i >= 0; i--) {
+				rtw_os_xmit_resource_free(padapter, pxmitbuf, (max_xmit_extbuf_size + XMITBUF_ALIGN_SZ));
+				pxmitbuf++;
+			}
+			goto free_xmit_extbuf;
 		}
 
 		list_add_tail(&pxmitbuf->list, &pxmitpriv->free_xmit_extbuf_queue.queue);
@@ -181,7 +190,7 @@ s32	_rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 
 	res = rtw_alloc_hwxmits(padapter);
 	if (res == _FAIL)
-		goto exit;
+		goto free_pxmit_extbuf;
 	rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
 
 	for (i = 0; i < 4; i++)
@@ -203,6 +212,26 @@ s32	_rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 
 	rtw_hal_init_xmit_priv(padapter);
 
+	return res;
+
+free_pxmit_extbuf:
+	pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmit_extbuf;
+	for (i = 0; i < num_xmit_extbuf; i++) {
+		rtw_os_xmit_resource_free(padapter, pxmitbuf, (max_xmit_extbuf_size + XMITBUF_ALIGN_SZ));
+		pxmitbuf++;
+	}
+free_xmit_extbuf:
+	vfree(pxmitpriv->pallocated_xmit_extbuf);
+free_pxmitbuf:
+	pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
+	for (i = 0; i < NR_XMITBUFF; i++) {
+		rtw_os_xmit_resource_free(padapter, pxmitbuf, (MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ));
+		pxmitbuf++;
+	}
+free_xmitbuf:
+	vfree(pxmitpriv->pallocated_xmitbuf);
+free_frame_buf:
+	vfree(pxmitpriv->pallocated_frame_buf);
 exit:
 	return res;
 }
diff --git a/drivers/staging/r8188eu/os_dep/xmit_linux.c b/drivers/staging/r8188eu/os_dep/xmit_linux.c
index 565ac5b..7aa39b5 100644
--- a/drivers/staging/r8188eu/os_dep/xmit_linux.c
+++ b/drivers/staging/r8188eu/os_dep/xmit_linux.c
@@ -95,8 +95,14 @@ void rtw_os_xmit_resource_free(struct adapter *padapter,
 {
 	int i;
 
-	for (i = 0; i < 8; i++)
+	if (!pxmitbuf->pallocated_buf)
+		return;
+
+	for (i = 0; i < 8; i++) {
+		if (!pxmitbuf->pxmit_urb[i])
+			break;
 		usb_free_urb(pxmitbuf->pxmit_urb[i]);
+	}
 
 	kfree(pxmitbuf->pallocated_buf);
 }
--