Re: [PATCH 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit_priv()

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Mar 30, 2022 at 11:29:22PM +0800, xkernel.wang@foxmail.com wrote:
> From: Xiaoke Wang <xkernel.wang@foxmail.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@foxmail.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++;
> +				}

This logic should be at the end of the function, when you are exiting
due to an error.  Don't duplicate it in multiple places, that way is
ensured to get out of sync eventually.

thanks,

greg k-h