All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: intel_sst: Fix memory leak
@ 2011-05-31 19:03 Andre Bartke
  2011-05-31 22:42 ` Dan Carpenter
  2011-06-01 12:50 ` Alan Cox
  0 siblings, 2 replies; 6+ messages in thread
From: Andre Bartke @ 2011-05-31 19:03 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, Andre Bartke

In case of an error stream_bufs is not freed here.

Signed-off-by: Andre Bartke <andre.bartke@gmail.com>
---
 .../staging/intel_sst/intel_sst_app_interface.c    |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/intel_sst/intel_sst_app_interface.c b/drivers/staging/intel_sst/intel_sst_app_interface.c
index b8c7ddb..93b41a2 100644
--- a/drivers/staging/intel_sst/intel_sst_app_interface.c
+++ b/drivers/staging/intel_sst/intel_sst_app_interface.c
@@ -430,8 +430,10 @@ static int snd_sst_fill_kernel_list(struct stream_info *stream,
 				return -ENOMEM;
 			if (copy_from_user((void *) &rar_handle,
 					iovec[index].iov_base,
-					sizeof(__u32)))
+					sizeof(__u32))) {
+				kfree(stream_bufs);
 				return -EFAULT;
+			}
 			stream_bufs->addr = (char *)rar_handle;
 			stream_bufs->in_use = false;
 			stream_bufs->size = iovec[0].iov_len;
-- 
1.7.5.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: intel_sst: Fix memory leak
  2011-05-31 19:03 [PATCH] staging: intel_sst: Fix memory leak Andre Bartke
@ 2011-05-31 22:42 ` Dan Carpenter
  2011-06-01 12:50 ` Alan Cox
  1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2011-05-31 22:42 UTC (permalink / raw)
  To: Andre Bartke; +Cc: gregkh, devel, Andre Bartke, linux-kernel

On Tue, May 31, 2011 at 09:03:56PM +0200, Andre Bartke wrote:
> In case of an error stream_bufs is not freed here.
> 
> Signed-off-by: Andre Bartke <andre.bartke@gmail.com>
> ---
>  .../staging/intel_sst/intel_sst_app_interface.c    |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/intel_sst/intel_sst_app_interface.c b/drivers/staging/intel_sst/intel_sst_app_interface.c
> index b8c7ddb..93b41a2 100644
> --- a/drivers/staging/intel_sst/intel_sst_app_interface.c
> +++ b/drivers/staging/intel_sst/intel_sst_app_interface.c
> @@ -430,8 +430,10 @@ static int snd_sst_fill_kernel_list(struct stream_info *stream,
>  				return -ENOMEM;
>  			if (copy_from_user((void *) &rar_handle,
>  					iovec[index].iov_base,
> -					sizeof(__u32)))
> +					sizeof(__u32))) {
> +				kfree(stream_bufs);
>  				return -EFAULT;
> +			}
>  			stream_bufs->addr = (char *)rar_handle;


Btw, the original code looks seems pretty awful.  "rar_handle" is
a kernel pointer that we get from user space.  It is a u32 type so
this won't work under a 64 bit OS.

These allocations get added to a list.  Maybe we should free them
all?  It's hard to say.  There is only one place that calls this
function, and it ignores the return value.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: intel_sst: Fix memory leak
  2011-05-31 19:03 [PATCH] staging: intel_sst: Fix memory leak Andre Bartke
  2011-05-31 22:42 ` Dan Carpenter
@ 2011-06-01 12:50 ` Alan Cox
  1 sibling, 0 replies; 6+ messages in thread
From: Alan Cox @ 2011-06-01 12:50 UTC (permalink / raw)
  To: Andre Bartke; +Cc: gregkh, devel, linux-kernel, Andre Bartke

On Tue, 31 May 2011 21:03:56 +0200
Andre Bartke <andre.bartke@googlemail.com> wrote:

> In case of an error stream_bufs is not freed here.
> 
> Signed-off-by: Andre Bartke <andre.bartke@gmail.com>

Acked-by: Alan Cox <alan@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch] Staging: intel_sst: fix memory leak
  2010-11-13  8:58 [patch] Staging: intel_sst: fix " Dan Carpenter
  2010-11-15  5:47 ` Harsha, Priya
@ 2010-11-16 13:38 ` Alan Cox
  1 sibling, 0 replies; 6+ messages in thread
From: Alan Cox @ 2010-11-16 13:38 UTC (permalink / raw)
  To: kernel-janitors

On Sat, 13 Nov 2010 11:58:26 +0300
Dan Carpenter <error27@gmail.com> wrote:

> The original code set "str_info->decode_ibuf" to NULL so the kfree()
> is no-op.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> This is a static checker thing.  I don't have the hardware to test
> this myself so please check it carefully.

Acked-by: Alan Cox <alan@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [patch] Staging: intel_sst: fix memory leak
  2010-11-13  8:58 [patch] Staging: intel_sst: fix " Dan Carpenter
@ 2010-11-15  5:47 ` Harsha, Priya
  2010-11-16 13:38 ` Alan Cox
  1 sibling, 0 replies; 6+ messages in thread
From: Harsha, Priya @ 2010-11-15  5:47 UTC (permalink / raw)
  To: kernel-janitors


> -----Original Message-----
> From: Dan Carpenter [mailto:error27@gmail.com]
> Sent: Saturday, November 13, 2010 2:28 PM
> To: Greg Kroah-Hartman
> Cc: Harsha, Priya; Koul, Vinod; Alan Cox; devel@driverdev.osuosl.org;
> kernel-janitors@vger.kernel.org
> Subject: [patch] Staging: intel_sst: fix memory leak
> 
> The original code set "str_info->decode_ibuf" to NULL so the kfree() is
> no-op.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> This is a static checker thing.  I don't have the hardware to test this
> myself so please check it carefully.
> 
> diff --git a/drivers/staging/intel_sst/intel_sst_stream_encoded.c
> b/drivers/staging/intel_sst/intel_sst_stream_encoded.c
> index d4e94f1..09993d6 100644
> --- a/drivers/staging/intel_sst/intel_sst_stream_encoded.c
> +++ b/drivers/staging/intel_sst/intel_sst_stream_encoded.c
> @@ -1270,7 +1270,7 @@ finish:
>  	dbufs->output_bytes_produced = total_output;
>  	str_info->status = str_info->prev;
>  	str_info->prev = STREAM_DECODE;
> -	str_info->decode_ibuf = NULL;
>  	kfree(str_info->decode_ibuf);
> +	str_info->decode_ibuf = NULL;
>  	return retval;
>  }
Acked-by: Harsha Priya <priya.harsha@intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [patch] Staging: intel_sst: fix memory leak
@ 2010-11-13  8:58 Dan Carpenter
  2010-11-15  5:47 ` Harsha, Priya
  2010-11-16 13:38 ` Alan Cox
  0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2010-11-13  8:58 UTC (permalink / raw)
  To: kernel-janitors

The original code set "str_info->decode_ibuf" to NULL so the kfree() is
no-op.

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
This is a static checker thing.  I don't have the hardware to test this
myself so please check it carefully.

diff --git a/drivers/staging/intel_sst/intel_sst_stream_encoded.c b/drivers/staging/intel_sst/intel_sst_stream_encoded.c
index d4e94f1..09993d6 100644
--- a/drivers/staging/intel_sst/intel_sst_stream_encoded.c
+++ b/drivers/staging/intel_sst/intel_sst_stream_encoded.c
@@ -1270,7 +1270,7 @@ finish:
 	dbufs->output_bytes_produced = total_output;
 	str_info->status = str_info->prev;
 	str_info->prev = STREAM_DECODE;
-	str_info->decode_ibuf = NULL;
 	kfree(str_info->decode_ibuf);
+	str_info->decode_ibuf = NULL;
 	return retval;
 }

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-06-01 12:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-31 19:03 [PATCH] staging: intel_sst: Fix memory leak Andre Bartke
2011-05-31 22:42 ` Dan Carpenter
2011-06-01 12:50 ` Alan Cox
  -- strict thread matches above, loose matches on Subject: below --
2010-11-13  8:58 [patch] Staging: intel_sst: fix " Dan Carpenter
2010-11-15  5:47 ` Harsha, Priya
2010-11-16 13:38 ` Alan Cox

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.