* [PATCH v2] ALSA: pcm: fix multiple memory leak bugs
@ 2019-08-07 7:09 Wenwen Wang
2019-08-07 7:18 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Wenwen Wang @ 2019-08-07 7:09 UTC (permalink / raw)
To: Wenwen Wang
Cc: Jaroslav Kysela, Takashi Iwai, Allison Randal, Thomas Gleixner,
Greg Kroah-Hartman, moderated list:SOUND, open list
In hiface_pcm_init(), 'rt' is firstly allocated through kzalloc(). Later
on, hiface_pcm_init_urb() is invoked to initialize 'rt->out_urbs[i]'. In
hiface_pcm_init_urb(), 'rt->out_urbs[i].buffer' is allocated through
kzalloc(). However, if hiface_pcm_init_urb() fails, both 'rt' and
'rt->out_urbs[i].buffer' are not deallocated, leading to memory leak bugs.
Also, 'rt->out_urbs[i].buffer' is not deallocated if snd_pcm_new() fails.
To fix the above issues, free 'rt' and 'rt->out_urbs[i].buffer'.
Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
---
sound/usb/hiface/pcm.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
index 14fc1e1..9b132aa 100644
--- a/sound/usb/hiface/pcm.c
+++ b/sound/usb/hiface/pcm.c
@@ -599,12 +599,18 @@ int hiface_pcm_init(struct hiface_chip *chip, u8
extra_freq)
for (i = 0; i < PCM_N_URBS; i++) {
ret = hiface_pcm_init_urb(&rt->out_urbs[i], chip, OUT_EP,
hiface_pcm_out_urb_handler);
- if (ret < 0)
+ if (ret < 0) {
+ for (; i >= 0; i--)
+ kfree(rt->out_urbs[i].buffer);
+ kfree(rt);
return ret;
+ }
}
ret = snd_pcm_new(chip->card, "USB-SPDIF Audio", 0, 1, 0, &pcm);
if (ret < 0) {
+ for (i = 0; i < PCM_N_URBS; i++)
+ kfree(rt->out_urbs[i].buffer);
kfree(rt);
dev_err(&chip->dev->dev, "Cannot create pcm instance\n");
return ret;
--
2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ALSA: pcm: fix multiple memory leak bugs
2019-08-07 7:09 [PATCH v2] ALSA: pcm: fix multiple memory leak bugs Wenwen Wang
@ 2019-08-07 7:18 ` Takashi Iwai
2019-08-07 7:24 ` Wenwen Wang
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2019-08-07 7:18 UTC (permalink / raw)
To: Wenwen Wang
Cc: moderated list:SOUND, Thomas Gleixner, Greg Kroah-Hartman,
Allison Randal, Jaroslav Kysela, Takashi Iwai, open list
On Wed, 07 Aug 2019 09:09:59 +0200,
Wenwen Wang wrote:
>
> In hiface_pcm_init(), 'rt' is firstly allocated through kzalloc(). Later
> on, hiface_pcm_init_urb() is invoked to initialize 'rt->out_urbs[i]'. In
> hiface_pcm_init_urb(), 'rt->out_urbs[i].buffer' is allocated through
> kzalloc(). However, if hiface_pcm_init_urb() fails, both 'rt' and
> 'rt->out_urbs[i].buffer' are not deallocated, leading to memory leak bugs.
> Also, 'rt->out_urbs[i].buffer' is not deallocated if snd_pcm_new() fails.
>
> To fix the above issues, free 'rt' and 'rt->out_urbs[i].buffer'.
>
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> ---
> sound/usb/hiface/pcm.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
> index 14fc1e1..9b132aa 100644
> --- a/sound/usb/hiface/pcm.c
> +++ b/sound/usb/hiface/pcm.c
> @@ -599,12 +599,18 @@ int hiface_pcm_init(struct hiface_chip *chip, u8
> extra_freq)
> for (i = 0; i < PCM_N_URBS; i++) {
> ret = hiface_pcm_init_urb(&rt->out_urbs[i], chip, OUT_EP,
> hiface_pcm_out_urb_handler);
> - if (ret < 0)
> + if (ret < 0) {
> + for (; i >= 0; i--)
> + kfree(rt->out_urbs[i].buffer);
> + kfree(rt);
> return ret;
> + }
> }
>
> ret = snd_pcm_new(chip->card, "USB-SPDIF Audio", 0, 1, 0, &pcm);
> if (ret < 0) {
> + for (i = 0; i < PCM_N_URBS; i++)
> + kfree(rt->out_urbs[i].buffer);
> kfree(rt);
> dev_err(&chip->dev->dev, "Cannot create pcm instance\n");
> return ret;
The fixes look correct, but since we can unconditionally call kfree()
for NULL, both error paths can be unified as:
for (i = 0; i < PCM_N_URBS; i++)
kfree(rt->out_urbs[i].buffer);
kfree(rt);
and this would be better to be put in the common path at the end and
do "goto error" or such from both places.
BTW, your patch doesn't seem cleanly applicable in anyway because the
tabs are converted to spaces. Please check the mail setup.
Also, please try to make the subject line more unique. This is about
hiface driver, so "ALSA: hiface: xxx" should be more appropriate.
thanks,
Takashi
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ALSA: pcm: fix multiple memory leak bugs
2019-08-07 7:18 ` Takashi Iwai
@ 2019-08-07 7:24 ` Wenwen Wang
0 siblings, 0 replies; 3+ messages in thread
From: Wenwen Wang @ 2019-08-07 7:24 UTC (permalink / raw)
To: Takashi Iwai
Cc: moderated list:SOUND, Thomas Gleixner, Greg Kroah-Hartman,
Allison Randal, Jaroslav Kysela, Takashi Iwai, open list,
Wenwen Wang
On Wed, Aug 7, 2019 at 3:18 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Wed, 07 Aug 2019 09:09:59 +0200,
> Wenwen Wang wrote:
> >
> > In hiface_pcm_init(), 'rt' is firstly allocated through kzalloc(). Later
> > on, hiface_pcm_init_urb() is invoked to initialize 'rt->out_urbs[i]'. In
> > hiface_pcm_init_urb(), 'rt->out_urbs[i].buffer' is allocated through
> > kzalloc(). However, if hiface_pcm_init_urb() fails, both 'rt' and
> > 'rt->out_urbs[i].buffer' are not deallocated, leading to memory leak bugs.
> > Also, 'rt->out_urbs[i].buffer' is not deallocated if snd_pcm_new() fails.
> >
> > To fix the above issues, free 'rt' and 'rt->out_urbs[i].buffer'.
> >
> > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> > ---
> > sound/usb/hiface/pcm.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
> > index 14fc1e1..9b132aa 100644
> > --- a/sound/usb/hiface/pcm.c
> > +++ b/sound/usb/hiface/pcm.c
> > @@ -599,12 +599,18 @@ int hiface_pcm_init(struct hiface_chip *chip, u8
> > extra_freq)
> > for (i = 0; i < PCM_N_URBS; i++) {
> > ret = hiface_pcm_init_urb(&rt->out_urbs[i], chip, OUT_EP,
> > hiface_pcm_out_urb_handler);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + for (; i >= 0; i--)
> > + kfree(rt->out_urbs[i].buffer);
> > + kfree(rt);
> > return ret;
> > + }
> > }
> >
> > ret = snd_pcm_new(chip->card, "USB-SPDIF Audio", 0, 1, 0, &pcm);
> > if (ret < 0) {
> > + for (i = 0; i < PCM_N_URBS; i++)
> > + kfree(rt->out_urbs[i].buffer);
> > kfree(rt);
> > dev_err(&chip->dev->dev, "Cannot create pcm instance\n");
> > return ret;
>
> The fixes look correct, but since we can unconditionally call kfree()
> for NULL, both error paths can be unified as:
>
> for (i = 0; i < PCM_N_URBS; i++)
> kfree(rt->out_urbs[i].buffer);
> kfree(rt);
>
> and this would be better to be put in the common path at the end and
> do "goto error" or such from both places.
I will rework the patch and revise the subject line.
> BTW, your patch doesn't seem cleanly applicable in anyway because the
> tabs are converted to spaces. Please check the mail setup.
>
> Also, please try to make the subject line more unique. This is about
> hiface driver, so "ALSA: hiface: xxx" should be more appropriate.
I will also check my mail setup. Thanks!
Wenwen
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-07 7:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07 7:09 [PATCH v2] ALSA: pcm: fix multiple memory leak bugs Wenwen Wang
2019-08-07 7:18 ` Takashi Iwai
2019-08-07 7:24 ` Wenwen Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®