lkml.org 
[lkml]   [2022]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] staging: rtl8712: fix potential memory leak in _r8712_init_xmit_priv()
On Mon, Sep 26, 2022 at 03:06:05PM +0800, xkernel.wang@foxmail.com wrote:
> diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c b/drivers/staging/rtl8712/rtl871x_xmit.c
> index 090345b..dcf3f76 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 free_frame_buf;
> pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
> ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
> pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
> @@ -130,12 +127,14 @@ 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 free_xmitbuf;
> 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;
> + if (r8712_xmit_resource_alloc(padapter, pxmitbuf)) {
> + kfree(pxmitbuf->pallocated_buf);
> + goto free_xmitbuf;
> + }
> list_add_tail(&pxmitbuf->list,
> &(pxmitpriv->free_xmitbuf_queue.queue));


pxmitbuf points to somewhere in the middle of pxmitpriv->pallocated_xmitbuf.
We add it to the list here.

> pxmitbuf++;
> @@ -146,6 +145,18 @@ 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;
> +
> +free_xmitbuf:
> + pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
> + while (i--) {
> + r8712_xmit_resource_free(padapter, pxmitbuf);
> + kfree(pxmitbuf->pallocated_buf);
> + pxmitbuf++;
> + }
> + kfree(pxmitpriv->pallocated_xmitbuf);

But then we free pxmitpriv->pallocated_xmitbuf here but it the memory
is still on the list. So that means there will be a use after free
eventually.

regards,
dan carpenter

\
 
 \ /
  Last update: 2022-09-27 10:05    [W:0.033 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site