On Wed, Jul 12, 2023 at 03:57:32PM -0500, Larry Finger wrote:
> In the above mentioned routine, memory is allocated in several places.
> If the first succeeds and a later one fails, the routine will leak memory.
> Fixes commit 2865d42c78a9 ("staging: r8712u: Add the new driver to the
> mainline kernel").
>
> Fixes: 2865d42c78a9 ("staging: r8712u: Add the new driver to the mainline kernel")
> Reported-by: syzbot+cf71097ffb6755df8251@xxxxxxxxxxxxxxxxxxxxxxxxx
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> ---
> drivers/staging/rtl8712/rtl871x_xmit.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c b/drivers/staging/rtl8712/rtl871x_xmit.c
> index 090345bad223..16b815588b97 100644
> --- a/drivers/staging/rtl8712/rtl871x_xmit.c
> +++ b/drivers/staging/rtl8712/rtl871x_xmit.c
> @@ -117,11 +117,8 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
> _init_queue(&pxmitpriv->pending_xmitbuf_queue);
> pxmitpriv->pallocated_xmitbuf =
> kmalloc(NR_XMITBUFF * sizeof(struct xmit_buf) + 4, GFP_ATOMIC);
> - if (!pxmitpriv->pallocated_xmitbuf) {
> - kfree(pxmitpriv->pallocated_frame_buf);
> - pxmitpriv->pallocated_frame_buf = NULL;
> - return -ENOMEM;
> - }
> + if (!pxmitpriv->pallocated_xmitbuf)
> + goto clean_up_frame_buf;
> pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
> ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
> pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
> @@ -130,12 +127,12 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
> pxmitbuf->pallocated_buf =
> kmalloc(MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ, GFP_ATOMIC);
> if (!pxmitbuf->pallocated_buf)
> - return -ENOMEM;
> + goto clean_up_xmit_buf;
> pxmitbuf->pbuf = pxmitbuf->pallocated_buf + XMITBUF_ALIGN_SZ -
> ((addr_t) (pxmitbuf->pallocated_buf) &
> (XMITBUF_ALIGN_SZ - 1));
> if (r8712_xmit_resource_alloc(padapter, pxmitbuf))
> - return -ENOMEM;
> + goto clean_up_xmit_buf;
> list_add_tail(&pxmitbuf->list,
> &(pxmitpriv->free_xmitbuf_queue.queue));
> pxmitbuf++;
> @@ -146,6 +143,14 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
> init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
> tasklet_setup(&pxmitpriv->xmit_tasklet, r8712_xmit_bh);
> return 0;
> +
> +clean_up_xmit_buf:
> + kfree(pxmitbuf->pallocated_xmitbuf);
> + pxmitbuf->pallocated_buf = NULL;
The allocation was done in a loop. Shouldn't memory from previous loop iterations
also be freed? And allocation by r8712_xmit_resource_alloc() should be freed too.
Best regards,
Nam
