2022-04-01 13:19:20

by Xiaoke Wang

Subject: [PATCH 2/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.
So it is better to check the return status of them.

Signed-off-by: Xiaoke Wang <[email protected]>
---
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 9502f6a..163baaa 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)
return ret;
- _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
- _r8712_init_recv_priv(&padapter->recvpriv, padapter);
+ ret = _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
+ if (ret)
+ return ret;
+ ret = _r8712_init_recv_priv(&padapter->recvpriv, padapter);
+ if (ret)
+ return ret;
memset((unsigned char *)&padapter->securitypriv, 0,
sizeof(struct security_priv));
timer_setup(&padapter->securitypriv.tkip_timer,
--

2022-04-05 01:32:19

by Greg Kroah-Hartman

Subject: Re: [PATCH 2/3] staging: rtl8712: add two validation check in r8712_init_drv_sw()

On Fri, Apr 01, 2022 at 01:07:45AM +0800, [email protected] wrote:
> From: Xiaoke Wang <[email protected]>
>
> _r8712_init_xmit_priv() or _r8712_init_recv_priv() returns -ENOMEM
> when some allocations inside it failed.
> So it is better to check the return status of them.
>
> Signed-off-by: Xiaoke Wang <[email protected]>
> ---
> 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 9502f6a..163baaa 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)
> return ret;
> - _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
> - _r8712_init_recv_priv(&padapter->recvpriv, padapter);
> + ret = _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
> + if (ret)
> + return ret;
> + ret = _r8712_init_recv_priv(&padapter->recvpriv, padapter);
> + if (ret)
> + return ret;

You just leaked memory :(

please please please test these types of "fix up error handling"
changes, as there are lots and lots of ways to get these wrong.

If you can not test them, provide some sort of proof that the change is
correct please.

thanks,

greg k-h

2022-04-05 03:42:46

by Xiaoke Wang

Subject: Re: [PATCH 2/3] staging: rtl8712: add two validation check in r8712_init_drv_sw()

On Monday, April 4, 2022 10:30 PM +0800, [email protected] wrote:
> You just leaked memory :(
>
> please please please test these types of "fix up error handling"
> changes, as there are lots and lots of ways to get these wrong.
>

It seems that you missed the third email in this series.

In fact, I did not ignore the memory leak problem. But considering that there are other paths which can also lead to memory leak, I separated another patch to deal with them specifically.

In the next version, I will adjust the sequence of the patches and add some descriptions to explicitly show the relations.

--
Regards,
Xiaoke Wang