* [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails
@ 2011-07-28 19:48 Jesper Juhl
2011-07-29 5:36 ` Takashi Iwai
0 siblings, 1 reply; 7+ messages in thread
From: Jesper Juhl @ 2011-07-28 19:48 UTC (permalink / raw)
To: linux-kernel
Cc: alsa-devel, Joe Perches, Eliot Blennerhassett, Takashi Iwai,
Jaroslav Kysela, support
We leak the memory allocated to 'firmware' when we fail to
release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
This patch should take care of the leak.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
sound/pci/asihpi/hpidspcd.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
Build tested only.
diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
index 3a7afa3..8d261ef 100644
--- a/sound/pci/asihpi/hpidspcd.c
+++ b/sound/pci/asihpi/hpidspcd.c
@@ -85,8 +85,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
- if (!dsp_code->pvt)
+ if (!dsp_code->pvt) {
+ release_firmware(firmware);
return HPI_ERROR_MEMORY_ALLOC;
+ }
dsp_code->pvt->dev = dev;
dsp_code->pvt->firmware = firmware;
--
1.7.6
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails
2011-07-28 19:48 [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails Jesper Juhl
@ 2011-07-29 5:36 ` Takashi Iwai
0 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2011-07-29 5:36 UTC (permalink / raw)
To: Jesper Juhl
Cc: linux-kernel, alsa-devel, Joe Perches, Eliot Blennerhassett,
Jaroslav Kysela, support
At Thu, 28 Jul 2011 21:48:27 +0200 (CEST),
Jesper Juhl wrote:
>
> We leak the memory allocated to 'firmware' when we fail to
> release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> This patch should take care of the leak.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
> sound/pci/asihpi/hpidspcd.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> Build tested only.
>
> diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> index 3a7afa3..8d261ef 100644
> --- a/sound/pci/asihpi/hpidspcd.c
> +++ b/sound/pci/asihpi/hpidspcd.c
> @@ -85,8 +85,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
>
> HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> - if (!dsp_code->pvt)
> + if (!dsp_code->pvt) {
> + release_firmware(firmware);
> return HPI_ERROR_MEMORY_ALLOC;
> + }
IMO, it's better to use the same error handling like others, i.e.
"goto error2". But you want to pass a different error code, so we'd
need to introduce an error code variable there...
Takashi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails
@ 2011-07-29 5:36 ` Takashi Iwai
0 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2011-07-29 5:36 UTC (permalink / raw)
To: Jesper Juhl
Cc: alsa-devel, linux-kernel, Eliot Blennerhassett, Joe Perches, support
At Thu, 28 Jul 2011 21:48:27 +0200 (CEST),
Jesper Juhl wrote:
>
> We leak the memory allocated to 'firmware' when we fail to
> release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> This patch should take care of the leak.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
> sound/pci/asihpi/hpidspcd.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> Build tested only.
>
> diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> index 3a7afa3..8d261ef 100644
> --- a/sound/pci/asihpi/hpidspcd.c
> +++ b/sound/pci/asihpi/hpidspcd.c
> @@ -85,8 +85,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
>
> HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> - if (!dsp_code->pvt)
> + if (!dsp_code->pvt) {
> + release_firmware(firmware);
> return HPI_ERROR_MEMORY_ALLOC;
> + }
IMO, it's better to use the same error handling like others, i.e.
"goto error2". But you want to pass a different error code, so we'd
need to introduce an error code variable there...
Takashi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails
2011-07-29 5:36 ` Takashi Iwai
(?)
@ 2011-07-29 9:07 ` Jesper Juhl
2011-07-31 21:16 ` Jesper Juhl
-1 siblings, 1 reply; 7+ messages in thread
From: Jesper Juhl @ 2011-07-29 9:07 UTC (permalink / raw)
To: Takashi Iwai
Cc: linux-kernel, alsa-devel, Joe Perches, Eliot Blennerhassett,
Jaroslav Kysela, support
On Fri, 29 Jul 2011, Takashi Iwai wrote:
> At Thu, 28 Jul 2011 21:48:27 +0200 (CEST),
> Jesper Juhl wrote:
> >
> > We leak the memory allocated to 'firmware' when we fail to
> > release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> > This patch should take care of the leak.
> >
> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > ---
> > sound/pci/asihpi/hpidspcd.c | 4 +++-
> > 1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > Build tested only.
> >
> > diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> > index 3a7afa3..8d261ef 100644
> > --- a/sound/pci/asihpi/hpidspcd.c
> > +++ b/sound/pci/asihpi/hpidspcd.c
> > @@ -85,8 +85,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
> >
> > HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> > dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> > - if (!dsp_code->pvt)
> > + if (!dsp_code->pvt) {
> > + release_firmware(firmware);
> > return HPI_ERROR_MEMORY_ALLOC;
> > + }
>
> IMO, it's better to use the same error handling like others, i.e.
> "goto error2". But you want to pass a different error code, so we'd
> need to introduce an error code variable there...
>
Thank you for the feedback. I'll cook up a better patch tonight.
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails
2011-07-29 9:07 ` Jesper Juhl
@ 2011-07-31 21:16 ` Jesper Juhl
2011-08-01 9:04 ` Takashi Iwai
0 siblings, 1 reply; 7+ messages in thread
From: Jesper Juhl @ 2011-07-31 21:16 UTC (permalink / raw)
To: Takashi Iwai
Cc: linux-kernel, alsa-devel, Joe Perches, Eliot Blennerhassett,
Jaroslav Kysela, support
On Fri, 29 Jul 2011, Jesper Juhl wrote:
> On Fri, 29 Jul 2011, Takashi Iwai wrote:
>
> > At Thu, 28 Jul 2011 21:48:27 +0200 (CEST),
> > Jesper Juhl wrote:
> > >
> > > We leak the memory allocated to 'firmware' when we fail to
> > > release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> > > This patch should take care of the leak.
> > >
> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > ---
> > > sound/pci/asihpi/hpidspcd.c | 4 +++-
> > > 1 files changed, 3 insertions(+), 1 deletions(-)
> > >
> > > Build tested only.
> > >
> > > diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> > > index 3a7afa3..8d261ef 100644
> > > --- a/sound/pci/asihpi/hpidspcd.c
> > > +++ b/sound/pci/asihpi/hpidspcd.c
> > > @@ -85,8 +85,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
> > >
> > > HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> > > dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> > > - if (!dsp_code->pvt)
> > > + if (!dsp_code->pvt) {
> > > + release_firmware(firmware);
> > > return HPI_ERROR_MEMORY_ALLOC;
> > > + }
> >
> > IMO, it's better to use the same error handling like others, i.e.
> > "goto error2". But you want to pass a different error code, so we'd
> > need to introduce an error code variable there...
> >
> Thank you for the feedback. I'll cook up a better patch tonight.
>
>
Sorry about the delay - had non-computer things to attend to.
I hope this is more to your liking :-)
From: Jesper Juhl <jj@chaosbits.net>
We leak the memory allocated to 'firmware' when we fail to
release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
This patch should take care of the leak.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
sound/pci/asihpi/hpidspcd.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
index 3a7afa3..71d32c8 100644
--- a/sound/pci/asihpi/hpidspcd.c
+++ b/sound/pci/asihpi/hpidspcd.c
@@ -43,6 +43,7 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
struct pci_dev *dev = os_data;
struct code_header header;
char fw_name[20];
+ short err_ret = HPI_ERROR_DSP_FILE_NOT_FOUND;
int err;
sprintf(fw_name, "asihpi/dsp%04x.bin", adapter);
@@ -85,8 +86,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
- if (!dsp_code->pvt)
- return HPI_ERROR_MEMORY_ALLOC;
+ if (!dsp_code->pvt) {
+ err_ret = HPI_ERROR_MEMORY_ALLOC;
+ goto error2;
+ }
dsp_code->pvt->dev = dev;
dsp_code->pvt->firmware = firmware;
@@ -99,7 +102,7 @@ error2:
release_firmware(firmware);
error1:
dsp_code->block_length = 0;
- return HPI_ERROR_DSP_FILE_NOT_FOUND;
+ return err_ret;
}
/*-------------------------------------------------------------------*/
--
1.7.6
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails
2011-07-31 21:16 ` Jesper Juhl
@ 2011-08-01 9:04 ` Takashi Iwai
0 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2011-08-01 9:04 UTC (permalink / raw)
To: Jesper Juhl
Cc: linux-kernel, alsa-devel, Joe Perches, Eliot Blennerhassett,
Jaroslav Kysela, support
At Sun, 31 Jul 2011 23:16:43 +0200 (CEST),
Jesper Juhl wrote:
>
> On Fri, 29 Jul 2011, Jesper Juhl wrote:
>
> > On Fri, 29 Jul 2011, Takashi Iwai wrote:
> >
> > > At Thu, 28 Jul 2011 21:48:27 +0200 (CEST),
> > > Jesper Juhl wrote:
> > > >
> > > > We leak the memory allocated to 'firmware' when we fail to
> > > > release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> > > > This patch should take care of the leak.
> > > >
> > > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > > ---
> > > > sound/pci/asihpi/hpidspcd.c | 4 +++-
> > > > 1 files changed, 3 insertions(+), 1 deletions(-)
> > > >
> > > > Build tested only.
> > > >
> > > > diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> > > > index 3a7afa3..8d261ef 100644
> > > > --- a/sound/pci/asihpi/hpidspcd.c
> > > > +++ b/sound/pci/asihpi/hpidspcd.c
> > > > @@ -85,8 +85,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
> > > >
> > > > HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> > > > dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> > > > - if (!dsp_code->pvt)
> > > > + if (!dsp_code->pvt) {
> > > > + release_firmware(firmware);
> > > > return HPI_ERROR_MEMORY_ALLOC;
> > > > + }
> > >
> > > IMO, it's better to use the same error handling like others, i.e.
> > > "goto error2". But you want to pass a different error code, so we'd
> > > need to introduce an error code variable there...
> > >
> > Thank you for the feedback. I'll cook up a better patch tonight.
> >
> >
>
> Sorry about the delay - had non-computer things to attend to.
>
> I hope this is more to your liking :-)
>
>
> From: Jesper Juhl <jj@chaosbits.net>
>
> We leak the memory allocated to 'firmware' when we fail to
> release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> This patch should take care of the leak.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Applied now. Thanks.
Takashi
> ---
> sound/pci/asihpi/hpidspcd.c | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> index 3a7afa3..71d32c8 100644
> --- a/sound/pci/asihpi/hpidspcd.c
> +++ b/sound/pci/asihpi/hpidspcd.c
> @@ -43,6 +43,7 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
> struct pci_dev *dev = os_data;
> struct code_header header;
> char fw_name[20];
> + short err_ret = HPI_ERROR_DSP_FILE_NOT_FOUND;
> int err;
>
> sprintf(fw_name, "asihpi/dsp%04x.bin", adapter);
> @@ -85,8 +86,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
>
> HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> - if (!dsp_code->pvt)
> - return HPI_ERROR_MEMORY_ALLOC;
> + if (!dsp_code->pvt) {
> + err_ret = HPI_ERROR_MEMORY_ALLOC;
> + goto error2;
> + }
>
> dsp_code->pvt->dev = dev;
> dsp_code->pvt->firmware = firmware;
> @@ -99,7 +102,7 @@ error2:
> release_firmware(firmware);
> error1:
> dsp_code->block_length = 0;
> - return HPI_ERROR_DSP_FILE_NOT_FOUND;
> + return err_ret;
> }
>
> /*-------------------------------------------------------------------*/
> --
> 1.7.6
>
>
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails
@ 2011-08-01 9:04 ` Takashi Iwai
0 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2011-08-01 9:04 UTC (permalink / raw)
To: Jesper Juhl
Cc: alsa-devel, linux-kernel, Eliot Blennerhassett, Joe Perches, support
At Sun, 31 Jul 2011 23:16:43 +0200 (CEST),
Jesper Juhl wrote:
>
> On Fri, 29 Jul 2011, Jesper Juhl wrote:
>
> > On Fri, 29 Jul 2011, Takashi Iwai wrote:
> >
> > > At Thu, 28 Jul 2011 21:48:27 +0200 (CEST),
> > > Jesper Juhl wrote:
> > > >
> > > > We leak the memory allocated to 'firmware' when we fail to
> > > > release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> > > > This patch should take care of the leak.
> > > >
> > > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > > ---
> > > > sound/pci/asihpi/hpidspcd.c | 4 +++-
> > > > 1 files changed, 3 insertions(+), 1 deletions(-)
> > > >
> > > > Build tested only.
> > > >
> > > > diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> > > > index 3a7afa3..8d261ef 100644
> > > > --- a/sound/pci/asihpi/hpidspcd.c
> > > > +++ b/sound/pci/asihpi/hpidspcd.c
> > > > @@ -85,8 +85,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
> > > >
> > > > HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> > > > dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> > > > - if (!dsp_code->pvt)
> > > > + if (!dsp_code->pvt) {
> > > > + release_firmware(firmware);
> > > > return HPI_ERROR_MEMORY_ALLOC;
> > > > + }
> > >
> > > IMO, it's better to use the same error handling like others, i.e.
> > > "goto error2". But you want to pass a different error code, so we'd
> > > need to introduce an error code variable there...
> > >
> > Thank you for the feedback. I'll cook up a better patch tonight.
> >
> >
>
> Sorry about the delay - had non-computer things to attend to.
>
> I hope this is more to your liking :-)
>
>
> From: Jesper Juhl <jj@chaosbits.net>
>
> We leak the memory allocated to 'firmware' when we fail to
> release_firmware() after a kmalloc() failure in hpi_dsp_code_open().
> This patch should take care of the leak.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Applied now. Thanks.
Takashi
> ---
> sound/pci/asihpi/hpidspcd.c | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/sound/pci/asihpi/hpidspcd.c b/sound/pci/asihpi/hpidspcd.c
> index 3a7afa3..71d32c8 100644
> --- a/sound/pci/asihpi/hpidspcd.c
> +++ b/sound/pci/asihpi/hpidspcd.c
> @@ -43,6 +43,7 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
> struct pci_dev *dev = os_data;
> struct code_header header;
> char fw_name[20];
> + short err_ret = HPI_ERROR_DSP_FILE_NOT_FOUND;
> int err;
>
> sprintf(fw_name, "asihpi/dsp%04x.bin", adapter);
> @@ -85,8 +86,10 @@ short hpi_dsp_code_open(u32 adapter, void *os_data, struct dsp_code *dsp_code,
>
> HPI_DEBUG_LOG(DEBUG, "dsp code %s opened\n", fw_name);
> dsp_code->pvt = kmalloc(sizeof(*dsp_code->pvt), GFP_KERNEL);
> - if (!dsp_code->pvt)
> - return HPI_ERROR_MEMORY_ALLOC;
> + if (!dsp_code->pvt) {
> + err_ret = HPI_ERROR_MEMORY_ALLOC;
> + goto error2;
> + }
>
> dsp_code->pvt->dev = dev;
> dsp_code->pvt->firmware = firmware;
> @@ -99,7 +102,7 @@ error2:
> release_firmware(firmware);
> error1:
> dsp_code->block_length = 0;
> - return HPI_ERROR_DSP_FILE_NOT_FOUND;
> + return err_ret;
> }
>
> /*-------------------------------------------------------------------*/
> --
> 1.7.6
>
>
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-08-01 9:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-28 19:48 [PATCH] Sound: AudioScience HPI: Don't leak firmware if mem alloc fails Jesper Juhl
2011-07-29 5:36 ` Takashi Iwai
2011-07-29 5:36 ` Takashi Iwai
2011-07-29 9:07 ` Jesper Juhl
2011-07-31 21:16 ` Jesper Juhl
2011-08-01 9:04 ` Takashi Iwai
2011-08-01 9:04 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.