linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()
@ 2022-09-07  7:48 Jianglei Nie
  2022-09-07 11:42 ` Mathias Nyman
  0 siblings, 1 reply; 6+ messages in thread
From: Jianglei Nie @ 2022-09-07  7:48 UTC (permalink / raw)
  To: mathias.nyman, gregkh; +Cc: linux-usb, linux-kernel, Jianglei Nie

xhci_alloc_stream_info() allocates stream context array for stream_info
->stream_ctx_array with xhci_alloc_stream_ctx(). When some error occurs,
stream_info->stream_ctx_array is not released, which will lead to a
memory leak.

We can fix it by releasing the stream_info->stream_ctx_array with
xhci_free_stream_ctx() on the error path to avoid the potential memory
leak.

Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
---
 drivers/usb/host/xhci-mem.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8c19e151a945..9e56aa28efcd 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -641,7 +641,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 			num_stream_ctxs, &stream_info->ctx_array_dma,
 			mem_flags);
 	if (!stream_info->stream_ctx_array)
-		goto cleanup_ctx;
+		goto cleanup_ring_array;
 	memset(stream_info->stream_ctx_array, 0,
 			sizeof(struct xhci_stream_ctx)*num_stream_ctxs);
 
@@ -702,6 +702,11 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 	}
 	xhci_free_command(xhci, stream_info->free_streams_command);
 cleanup_ctx:
+	xhci_free_stream_ctx(xhci,
+		stream_info->num_stream_ctxs,
+		stream_info->stream_ctx_array,
+		stream_info->ctx_array_dma);
+cleanup_ring_array:
 	kfree(stream_info->stream_rings);
 cleanup_info:
 	kfree(stream_info);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()
  2022-09-07  7:48 [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info() Jianglei Nie
@ 2022-09-07 11:42 ` Mathias Nyman
  0 siblings, 0 replies; 6+ messages in thread
From: Mathias Nyman @ 2022-09-07 11:42 UTC (permalink / raw)
  To: Jianglei Nie, mathias.nyman, gregkh; +Cc: linux-usb, linux-kernel

On 7.9.2022 10.48, Jianglei Nie wrote:
> xhci_alloc_stream_info() allocates stream context array for stream_info
> ->stream_ctx_array with xhci_alloc_stream_ctx(). When some error occurs,
> stream_info->stream_ctx_array is not released, which will lead to a
> memory leak.
> 
> We can fix it by releasing the stream_info->stream_ctx_array with
> xhci_free_stream_ctx() on the error path to avoid the potential memory
> leak.
> 
> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>

Thanks, good to get this fixed.

Adding to for-usb-next and skipping stable as hitting this path is mostly theoretical.

-Mathias


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()
  2022-06-30  9:57 ` Mathias Nyman
@ 2022-06-30 10:09   ` Mathias Nyman
  0 siblings, 0 replies; 6+ messages in thread
From: Mathias Nyman @ 2022-06-30 10:09 UTC (permalink / raw)
  To: Jianglei Nie, mathias.nyman, gregkh; +Cc: linux-usb, linux-kernel

> 
> How about:
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 8c19e151a945..f7cac1af51c5 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -641,7 +641,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
>                          num_stream_ctxs, &stream_info->ctx_array_dma,
>                          mem_flags);
>          if (!stream_info->stream_ctx_array)
> -               goto cleanup_ctx;
> +               goto cleanup_rings;

Small sidenote, "cleanup_rings" label was already in use, so maybe something like
"cleanup_ring_array" could be used

Thanks
-Mathias



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()
  2022-06-30  1:10 Jianglei Nie
  2022-06-30  6:48 ` Greg KH
@ 2022-06-30  9:57 ` Mathias Nyman
  2022-06-30 10:09   ` Mathias Nyman
  1 sibling, 1 reply; 6+ messages in thread
From: Mathias Nyman @ 2022-06-30  9:57 UTC (permalink / raw)
  To: Jianglei Nie, mathias.nyman, gregkh; +Cc: linux-usb, linux-kernel

On 30.6.2022 4.10, Jianglei Nie wrote:
> xhci_alloc_stream_info() allocates stream context array for stream_info
> ->stream_ctx_array with xhci_alloc_stream_ctx(). When some error occurs,
> stream_info->stream_ctx_array is not released, which will lead to a
> memory leak.

Nice catch, thanks

> 
> We can fix it by releasing the stream_info->stream_ctx_array with
> xhci_free_stream_ctx() on the error path to avoid the potential memory
> leak.
> 

Looks like the goto labels were a bit messed up from the beginning.
There are a couple "goto cleanup_ctx" lines in the code, but
cleanup_ctx never freed the ctx.

> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
> ---
>   drivers/usb/host/xhci-mem.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 8c19e151a945..a71d3a873467 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -648,8 +648,13 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
>   	/* Allocate everything needed to free the stream rings later */
>   	stream_info->free_streams_command =
>   		xhci_alloc_command_with_ctx(xhci, true, mem_flags);
> -	if (!stream_info->free_streams_command)
> +	if (!stream_info->free_streams_command) {
> +		xhci_free_stream_ctx(xhci,
> +			stream_info->num_stream_ctxs,
> +			stream_info->stream_ctx_array,
> +			stream_info->ctx_array_dma);
>   		goto cleanup_ctx;
> +	}
>   
>   	INIT_RADIX_TREE(&stream_info->trb_address_map, GFP_ATOMIC);
>   
> @@ -700,6 +705,10 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
>   			stream_info->stream_rings[cur_stream] = NULL;
>   		}
>   	}
> +	xhci_free_stream_ctx(xhci,
> +			stream_info->num_stream_ctxs,
> +			stream_info->stream_ctx_array,
> +			stream_info->ctx_array_dma);
>   	xhci_free_command(xhci, stream_info->free_streams_command);
>   cleanup_ctx:
>   	kfree(stream_info->stream_rings);

How about:

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8c19e151a945..f7cac1af51c5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -641,7 +641,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
                         num_stream_ctxs, &stream_info->ctx_array_dma,
                         mem_flags);
         if (!stream_info->stream_ctx_array)
-               goto cleanup_ctx;
+               goto cleanup_rings;
         memset(stream_info->stream_ctx_array, 0,
                         sizeof(struct xhci_stream_ctx)*num_stream_ctxs);
  
@@ -702,6 +702,10 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
         }
         xhci_free_command(xhci, stream_info->free_streams_command);
  cleanup_ctx:
+       xhci_free_stream_ctx(xhci, stream_info->num_stream_ctxs,
+                            stream_info->stream_ctx_array,
+                            stream_info->ctx_array_dma);
+cleanup_rings:
         kfree(stream_info->stream_rings);
  cleanup_info:

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()
  2022-06-30  1:10 Jianglei Nie
@ 2022-06-30  6:48 ` Greg KH
  2022-06-30  9:57 ` Mathias Nyman
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2022-06-30  6:48 UTC (permalink / raw)
  To: Jianglei Nie; +Cc: mathias.nyman, linux-usb, linux-kernel

On Thu, Jun 30, 2022 at 09:10:08AM +0800, Jianglei Nie wrote:
> xhci_alloc_stream_info() allocates stream context array for stream_info
> ->stream_ctx_array with xhci_alloc_stream_ctx(). When some error occurs,
> stream_info->stream_ctx_array is not released, which will lead to a
> memory leak.
> 
> We can fix it by releasing the stream_info->stream_ctx_array with
> xhci_free_stream_ctx() on the error path to avoid the potential memory
> leak.
> 
> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
> ---
>  drivers/usb/host/xhci-mem.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

What commit id does this fix?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()
@ 2022-06-30  1:10 Jianglei Nie
  2022-06-30  6:48 ` Greg KH
  2022-06-30  9:57 ` Mathias Nyman
  0 siblings, 2 replies; 6+ messages in thread
From: Jianglei Nie @ 2022-06-30  1:10 UTC (permalink / raw)
  To: mathias.nyman, gregkh; +Cc: linux-usb, linux-kernel, Jianglei Nie

xhci_alloc_stream_info() allocates stream context array for stream_info
->stream_ctx_array with xhci_alloc_stream_ctx(). When some error occurs,
stream_info->stream_ctx_array is not released, which will lead to a
memory leak.

We can fix it by releasing the stream_info->stream_ctx_array with
xhci_free_stream_ctx() on the error path to avoid the potential memory
leak.

Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
---
 drivers/usb/host/xhci-mem.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8c19e151a945..a71d3a873467 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -648,8 +648,13 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 	/* Allocate everything needed to free the stream rings later */
 	stream_info->free_streams_command =
 		xhci_alloc_command_with_ctx(xhci, true, mem_flags);
-	if (!stream_info->free_streams_command)
+	if (!stream_info->free_streams_command) {
+		xhci_free_stream_ctx(xhci,
+			stream_info->num_stream_ctxs,
+			stream_info->stream_ctx_array,
+			stream_info->ctx_array_dma);
 		goto cleanup_ctx;
+	}
 
 	INIT_RADIX_TREE(&stream_info->trb_address_map, GFP_ATOMIC);
 
@@ -700,6 +705,10 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 			stream_info->stream_rings[cur_stream] = NULL;
 		}
 	}
+	xhci_free_stream_ctx(xhci,
+			stream_info->num_stream_ctxs,
+			stream_info->stream_ctx_array,
+			stream_info->ctx_array_dma);
 	xhci_free_command(xhci, stream_info->free_streams_command);
 cleanup_ctx:
 	kfree(stream_info->stream_rings);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-09-07 11:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-07  7:48 [PATCH] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info() Jianglei Nie
2022-09-07 11:42 ` Mathias Nyman
  -- strict thread matches above, loose matches on Subject: below --
2022-06-30  1:10 Jianglei Nie
2022-06-30  6:48 ` Greg KH
2022-06-30  9:57 ` Mathias Nyman
2022-06-30 10:09   ` Mathias Nyman

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).