* [PATCH] staging: 7811: Fix memory leak in _r8712_init_xmit_priv
@ 2023-07-12 20:57 Larry Finger
2023-07-13 17:35 ` Your Name
0 siblings, 1 reply; 4+ messages in thread
From: Larry Finger @ 2023-07-12 20:57 UTC (permalink / raw)
To: gregkh
Cc: linux-staging, linux-kernel, linux-wireless, Larry Finger,
syzbot+cf71097ffb6755df8251, stable
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@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
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;
+clean_up_frame_buf:
+ kfree(pxmitpriv->pallocated_frame_buf);
+ pxmitpriv->pallocated_frame_buf = NULL;
+ return -ENOMEM;
}
void _free_xmit_priv(struct xmit_priv *pxmitpriv)
--
2.41.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: 7811: Fix memory leak in _r8712_init_xmit_priv
2023-07-12 20:57 [PATCH] staging: 7811: Fix memory leak in _r8712_init_xmit_priv Larry Finger
@ 2023-07-13 17:35 ` Your Name
2023-07-13 18:51 ` Larry Finger
0 siblings, 1 reply; 4+ messages in thread
From: Your Name @ 2023-07-13 17:35 UTC (permalink / raw)
To: Larry Finger
Cc: gregkh, linux-staging, linux-kernel, linux-wireless,
syzbot+cf71097ffb6755df8251, stable
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@syzkaller.appspotmail.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: 7811: Fix memory leak in _r8712_init_xmit_priv
2023-07-13 17:35 ` Your Name
@ 2023-07-13 18:51 ` Larry Finger
2023-07-13 20:10 ` Nam Cao
0 siblings, 1 reply; 4+ messages in thread
From: Larry Finger @ 2023-07-13 18:51 UTC (permalink / raw)
To: Your Name
Cc: gregkh, linux-staging, linux-kernel, linux-wireless,
syzbot+cf71097ffb6755df8251, stable
On 7/13/23 12:35, Your Name wrote:
> 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.
Nam,
You are right on both counts. I will prepare version 2.
Larry
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: 7811: Fix memory leak in _r8712_init_xmit_priv
2023-07-13 18:51 ` Larry Finger
@ 2023-07-13 20:10 ` Nam Cao
0 siblings, 0 replies; 4+ messages in thread
From: Nam Cao @ 2023-07-13 20:10 UTC (permalink / raw)
To: Larry Finger
Cc: gregkh, linux-staging, linux-kernel, linux-wireless,
syzbot+cf71097ffb6755df8251, stable
On Thu, Jul 13, 2023 at 01:51:00PM -0500, Larry Finger wrote:
> On 7/13/23 12:35, Your Name wrote:
> > 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.
>
> Nam,
>
> You are right on both counts. I will prepare version 2.
>
> Larry
Perhaps you can make use of my WIP that I didn't have time to finish:
https://github.com/covanam/linux/commit/ee8a0719ec8535f932b30aa67325fc21ba998c50
(Untested and the label names are kind of dumb)
Best regards,
Nam
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-13 20:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-12 20:57 [PATCH] staging: 7811: Fix memory leak in _r8712_init_xmit_priv Larry Finger
2023-07-13 17:35 ` Your Name
2023-07-13 18:51 ` Larry Finger
2023-07-13 20:10 ` Nam Cao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).