2022-03-25 15:16:22

by Xiaoke Wang

Subject: [PATCH] staging: r8188eu: check the return value of kzalloc()

From: Xiaoke Wang <[email protected]>

kzalloc() is a memory allocation function which can return NULL when
some internal memory errors happen. So it is better to check the return
of it to prevent potential wrong memory access.

Signed-off-by: Xiaoke Wang <[email protected]>
---
drivers/staging/r8188eu/core/rtw_p2p.c | 2 ++
drivers/staging/r8188eu/core/rtw_xmit.c | 6 ++++++
2 files changed, 8 insertions(+)

diff --git a/drivers/staging/r8188eu/core/rtw_p2p.c b/drivers/staging/r8188eu/core/rtw_p2p.c
index e2b6cf2..503c4a5 100644
--- a/drivers/staging/r8188eu/core/rtw_p2p.c
+++ b/drivers/staging/r8188eu/core/rtw_p2p.c
@@ -35,6 +35,8 @@ static u32 go_add_group_info_attr(struct wifidirect_info *pwdinfo, u8 *pbuf)
DBG_88E("%s\n", __func__);

pdata_attr = kzalloc(MAX_P2P_IE_LEN, GFP_KERNEL);
+ if (!pdata_attr)
+ return 0;

pstart = pdata_attr;
pcur = pdata_attr;
diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 46fe62c..1696272 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -180,6 +180,10 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
pxmitpriv->free_xmit_extbuf_cnt = num_xmit_extbuf;

rtw_alloc_hwxmits(padapter);
+ if (!pxmitpriv->hwxmits) {
+ res = _FAIL;
+ goto exit;
+ }
rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);

for (i = 0; i < 4; i++)
@@ -1524,6 +1528,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
pxmitpriv->hwxmit_entry = HWXMIT_ENTRY;

pxmitpriv->hwxmits = kzalloc(sizeof(struct hw_xmit) * pxmitpriv->hwxmit_entry, GFP_KERNEL);
+ if (!pxmitpriv->hwxmits)
+ return;

hwxmits = pxmitpriv->hwxmits;

--

2022-03-31 04:08:34

by Greg Kroah-Hartman

Subject: Re: [PATCH] staging: r8188eu: check the return value of kzalloc()

On Fri, Mar 25, 2022 at 02:53:30PM +0800, [email protected] wrote:
> From: Xiaoke Wang <[email protected]>
>
> kzalloc() is a memory allocation function which can return NULL when
> some internal memory errors happen. So it is better to check the return
> of it to prevent potential wrong memory access.
>
> Signed-off-by: Xiaoke Wang <[email protected]>
> ---
> drivers/staging/r8188eu/core/rtw_p2p.c | 2 ++
> drivers/staging/r8188eu/core/rtw_xmit.c | 6 ++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_p2p.c b/drivers/staging/r8188eu/core/rtw_p2p.c
> index e2b6cf2..503c4a5 100644
> --- a/drivers/staging/r8188eu/core/rtw_p2p.c
> +++ b/drivers/staging/r8188eu/core/rtw_p2p.c
> @@ -35,6 +35,8 @@ static u32 go_add_group_info_attr(struct wifidirect_info *pwdinfo, u8 *pbuf)
> DBG_88E("%s\n", __func__);
>
> pdata_attr = kzalloc(MAX_P2P_IE_LEN, GFP_KERNEL);
> + if (!pdata_attr)
> + return 0;

0 is not an error. Please propagate this error backwards properly.

>
> pstart = pdata_attr;
> pcur = pdata_attr;
> diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
> index 46fe62c..1696272 100644
> --- a/drivers/staging/r8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/r8188eu/core/rtw_xmit.c
> @@ -180,6 +180,10 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
> pxmitpriv->free_xmit_extbuf_cnt = num_xmit_extbuf;
>
> rtw_alloc_hwxmits(padapter);
> + if (!pxmitpriv->hwxmits) {
> + res = _FAIL;
> + goto exit;
> + }

You just leaked memory resources :(

How did you test this?


> rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>
> for (i = 0; i < 4; i++)
> @@ -1524,6 +1528,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
> pxmitpriv->hwxmit_entry = HWXMIT_ENTRY;
>
> pxmitpriv->hwxmits = kzalloc(sizeof(struct hw_xmit) * pxmitpriv->hwxmit_entry, GFP_KERNEL);
> + if (!pxmitpriv->hwxmits)
> + return;

You have to return an error, you can not keep going as if all is well.

Please always be VERY careful with these types of fixes. Especially if
you have not tested them.

thanks,

greg k-h