2022-04-06 14:04:53

by Xiaoke Wang

[permalink] [raw]
Subject: [PATCH v3 1/3] staging: rtl8712: fix potential memory leak in r8712_init_drv_sw()

From: Xiaoke Wang <[email protected]>

In r8712_init_drv_sw(), there are various error paths do not properly
release the previous allocated resources but directly return the error
status, which leads to various memory leaks.

To properly release the resources, this patch unifies the error handler
of r8712_init_drv_sw().
According to the allocation sequence, if the init function returns
failure, it will jump to the corresponding error tag.

Signed-off-by: Xiaoke Wang <[email protected]>
---
ChangeLog:
v1->v2 adjust the sequence of the patches in this series.
v2->v3 update the description.
drivers/staging/rtl8712/os_intfs.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index 9502f6a..1f7ccec 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -304,10 +304,10 @@ int r8712_init_drv_sw(struct _adapter *padapter)
padapter->cmdpriv.padapter = padapter;
ret = r8712_init_evt_priv(&padapter->evtpriv);
if (ret)
- return ret;
+ goto free_cmd_priv;
ret = r8712_init_mlme_priv(padapter);
if (ret)
- return ret;
+ goto free_evt_priv;
_r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
_r8712_init_recv_priv(&padapter->recvpriv, padapter);
memset((unsigned char *)&padapter->securitypriv, 0,
@@ -316,13 +316,25 @@ int r8712_init_drv_sw(struct _adapter *padapter)
r8712_use_tkipkey_handler, 0);
ret = _r8712_init_sta_priv(&padapter->stapriv);
if (ret)
- return ret;
+ goto free_recv_priv;
padapter->stapriv.padapter = padapter;
r8712_init_bcmc_stainfo(padapter);
r8712_init_pwrctrl_priv(padapter);
mp871xinit(padapter);
init_default_value(padapter);
r8712_InitSwLeds(padapter);
+ return 0;
+
+free_recv_priv:
+ _r8712_free_recv_priv(&padapter->recvpriv);
+free_xmit_priv:
+ _free_xmit_priv(&padapter->xmitpriv);
+free_mlme_priv:
+ r8712_free_mlme_priv(&padapter->mlmepriv);
+free_evt_priv:
+ r8712_free_evt_priv(&padapter->evtpriv);
+free_cmd_priv:
+ r8712_free_cmd_priv(&padapter->cmdpriv);
return ret;
}

--


2022-04-06 14:56:52

by Xiaoke Wang

[permalink] [raw]
Subject: [PATCH v3 2/3] staging: rtl8712: change the type of _r8712_init_recv_priv()

From: Xiaoke Wang <[email protected]>

There is a memory allocation in _r8712_init_recv_priv().
However, since the original type of this function is `void`, its error
status can not be feedbacked to its caller.
Therefore, to make the error of allocation failures propagate to its
caller easily, this patch changes the type of this function to `int`.

Signed-off-by: Xiaoke Wang <[email protected]>
---
ChangeLog:
v1->v2 remove kmemleak_not_leak().
v2->v3 update the description.
drivers/staging/rtl8712/recv_osdep.h | 2 +-
drivers/staging/rtl8712/rtl871x_recv.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8712/recv_osdep.h b/drivers/staging/rtl8712/recv_osdep.h
index d8c1fa7..f5b97c5 100644
--- a/drivers/staging/rtl8712/recv_osdep.h
+++ b/drivers/staging/rtl8712/recv_osdep.h
@@ -18,7 +18,7 @@
#include "drv_types.h"
#include <linux/skbuff.h>

-void _r8712_init_recv_priv(struct recv_priv *precvpriv,
+int _r8712_init_recv_priv(struct recv_priv *precvpriv,
struct _adapter *padapter);
void _r8712_free_recv_priv(struct recv_priv *precvpriv);
void r8712_recv_entry(union recv_frame *precv_frame);
diff --git a/drivers/staging/rtl8712/rtl871x_recv.c b/drivers/staging/rtl8712/rtl871x_recv.c
index c23f6b3..3464ee2 100644
--- a/drivers/staging/rtl8712/rtl871x_recv.c
+++ b/drivers/staging/rtl8712/rtl871x_recv.c
@@ -44,7 +44,7 @@ void _r8712_init_sta_recv_priv(struct sta_recv_priv *psta_recvpriv)
_init_queue(&psta_recvpriv->defrag_q);
}

-void _r8712_init_recv_priv(struct recv_priv *precvpriv,
+int _r8712_init_recv_priv(struct recv_priv *precvpriv,
struct _adapter *padapter)
{
sint i;
@@ -60,8 +60,7 @@ void _r8712_init_recv_priv(struct recv_priv *precvpriv,
sizeof(union recv_frame) + RXFRAME_ALIGN_SZ,
GFP_ATOMIC);
if (!precvpriv->pallocated_frame_buf)
- return;
- kmemleak_not_leak(precvpriv->pallocated_frame_buf);
+ return -ENOMEM;
precvpriv->precv_frame_buf = precvpriv->pallocated_frame_buf +
RXFRAME_ALIGN_SZ -
((addr_t)(precvpriv->pallocated_frame_buf) &
@@ -77,6 +76,7 @@ void _r8712_init_recv_priv(struct recv_priv *precvpriv,
}
precvpriv->rx_pending_cnt = 1;
r8712_init_recv_priv(precvpriv, padapter);
+ return 0;
}

void _r8712_free_recv_priv(struct recv_priv *precvpriv)
--

2022-04-06 15:26:16

by Xiaoke Wang

[permalink] [raw]
Subject: [PATCH v3 3/3] staging: rtl8712: add two validation check in r8712_init_drv_sw()

From: Xiaoke Wang <[email protected]>

_r8712_init_xmit_priv() or _r8712_init_recv_priv() returns -ENOMEM
when some allocations inside it failed.
However, the caller, i.e., r8712_init_drv_sw(), does not properly
validate their return status, which may lead to potential wrong memory
access in the future.

Therefore, this patch adds two validation check for their return status
and properly jump to the corresponding error hanlding code if failures
happen.

Signed-off-by: Xiaoke Wang <[email protected]>
---
ChangeLog:
v1->v2 adjust the sequence of patches in this series.
v2->v3 update the description.
drivers/staging/rtl8712/os_intfs.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index 1f7ccec..0dbf8c2 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -308,8 +308,12 @@ int r8712_init_drv_sw(struct _adapter *padapter)
ret = r8712_init_mlme_priv(padapter);
if (ret)
goto free_evt_priv;
- _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
- _r8712_init_recv_priv(&padapter->recvpriv, padapter);
+ ret = _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
+ if (ret)
+ goto free_mlme_priv;
+ ret = _r8712_init_recv_priv(&padapter->recvpriv, padapter);
+ if (ret)
+ goto free_xmit_priv;
memset((unsigned char *)&padapter->securitypriv, 0,
sizeof(struct security_priv));
timer_setup(&padapter->securitypriv.tkip_timer,
--