* [PATCH] sound: Fix memory leak on error in snd_compr_set_params()
@ 2012-01-23 20:02 Jesper Juhl
2012-01-24 9:16 ` Vinod Koul
2012-01-24 14:30 ` Takashi Iwai
0 siblings, 2 replies; 3+ messages in thread
From: Jesper Juhl @ 2012-01-23 20:02 UTC (permalink / raw)
To: Takashi Iwai
Cc: linux-kernel, alsa-devel, Vinod Koul, Pierre-Louis Bossart,
Jaroslav Kysela
If copy_from_user() does not return 0 we'll leak the memory we
allocated for 'params' when that variable goes out of scope.
Also a small CodingStyle cleanup: Use braces on both branches of
if/else when one branch needs it.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
sound/core/compress_offload.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index dac3633..a68aed7 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -441,19 +441,22 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
params = kmalloc(sizeof(*params), GFP_KERNEL);
if (!params)
return -ENOMEM;
- if (copy_from_user(params, (void __user *)arg, sizeof(*params)))
- return -EFAULT;
+ if (copy_from_user(params, (void __user *)arg, sizeof(*params))) {
+ retval = -EFAULT;
+ goto out;
+ }
retval = snd_compr_allocate_buffer(stream, params);
if (retval) {
- kfree(params);
- return -ENOMEM;
+ retval = -ENOMEM;
+ goto out;
}
retval = stream->ops->set_params(stream, params);
if (retval)
goto out;
stream->runtime->state = SNDRV_PCM_STATE_SETUP;
- } else
+ } else {
return -EPERM;
+ }
out:
kfree(params);
return retval;
--
1.7.8.4
--
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] 3+ messages in thread
* Re: [PATCH] sound: Fix memory leak on error in snd_compr_set_params()
2012-01-23 20:02 [PATCH] sound: Fix memory leak on error in snd_compr_set_params() Jesper Juhl
@ 2012-01-24 9:16 ` Vinod Koul
2012-01-24 14:30 ` Takashi Iwai
1 sibling, 0 replies; 3+ messages in thread
From: Vinod Koul @ 2012-01-24 9:16 UTC (permalink / raw)
To: Jesper Juhl
Cc: Takashi Iwai, linux-kernel, alsa-devel, Pierre-Louis Bossart,
Jaroslav Kysela
On Mon, 2012-01-23 at 21:02 +0100, Jesper Juhl wrote:
> If copy_from_user() does not return 0 we'll leak the memory we
> allocated for 'params' when that variable goes out of scope.
>
> Also a small CodingStyle cleanup: Use braces on both branches of
> if/else when one branch needs it.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Thanks for the fix
Acked-by: Vinod Koul <vinod.koul@linux.intel.com>
> ---
> sound/core/compress_offload.c | 13 ++++++++-----
> 1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index dac3633..a68aed7 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -441,19 +441,22 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
> params = kmalloc(sizeof(*params), GFP_KERNEL);
> if (!params)
> return -ENOMEM;
> - if (copy_from_user(params, (void __user *)arg, sizeof(*params)))
> - return -EFAULT;
> + if (copy_from_user(params, (void __user *)arg, sizeof(*params))) {
> + retval = -EFAULT;
> + goto out;
> + }
> retval = snd_compr_allocate_buffer(stream, params);
> if (retval) {
> - kfree(params);
> - return -ENOMEM;
> + retval = -ENOMEM;
> + goto out;
> }
> retval = stream->ops->set_params(stream, params);
> if (retval)
> goto out;
> stream->runtime->state = SNDRV_PCM_STATE_SETUP;
> - } else
> + } else {
> return -EPERM;
> + }
> out:
> kfree(params);
> return retval;
> --
> 1.7.8.4
>
>
--
~Vinod
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sound: Fix memory leak on error in snd_compr_set_params()
2012-01-23 20:02 [PATCH] sound: Fix memory leak on error in snd_compr_set_params() Jesper Juhl
2012-01-24 9:16 ` Vinod Koul
@ 2012-01-24 14:30 ` Takashi Iwai
1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2012-01-24 14:30 UTC (permalink / raw)
To: Jesper Juhl
Cc: linux-kernel, alsa-devel, Vinod Koul, Pierre-Louis Bossart,
Jaroslav Kysela
At Mon, 23 Jan 2012 21:02:57 +0100 (CET),
Jesper Juhl wrote:
>
> If copy_from_user() does not return 0 we'll leak the memory we
> allocated for 'params' when that variable goes out of scope.
>
> Also a small CodingStyle cleanup: Use braces on both branches of
> if/else when one branch needs it.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Thanks, applied now.
Takashi
> ---
> sound/core/compress_offload.c | 13 ++++++++-----
> 1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index dac3633..a68aed7 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -441,19 +441,22 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
> params = kmalloc(sizeof(*params), GFP_KERNEL);
> if (!params)
> return -ENOMEM;
> - if (copy_from_user(params, (void __user *)arg, sizeof(*params)))
> - return -EFAULT;
> + if (copy_from_user(params, (void __user *)arg, sizeof(*params))) {
> + retval = -EFAULT;
> + goto out;
> + }
> retval = snd_compr_allocate_buffer(stream, params);
> if (retval) {
> - kfree(params);
> - return -ENOMEM;
> + retval = -ENOMEM;
> + goto out;
> }
> retval = stream->ops->set_params(stream, params);
> if (retval)
> goto out;
> stream->runtime->state = SNDRV_PCM_STATE_SETUP;
> - } else
> + } else {
> return -EPERM;
> + }
> out:
> kfree(params);
> return retval;
> --
> 1.7.8.4
>
>
> --
> 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] 3+ messages in thread
end of thread, other threads:[~2012-01-24 14:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-23 20:02 [PATCH] sound: Fix memory leak on error in snd_compr_set_params() Jesper Juhl
2012-01-24 9:16 ` Vinod Koul
2012-01-24 14:30 ` Takashi Iwai
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).