[PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[ant.v.moryakov]

From: Anton Moryakov <ant.v.moryakov@gmail.com>

Ensure int_queue is properly destroyed when receiving wrong buffer
by adding error handling path. Fixes memory leak that occurred
when backbuffer validation failed.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
 drivers/usb/host/ehci-hcd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 7d5519c65a9..765d8b327ee 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1557,6 +1557,7 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 		debug("got wrong buffer back (%p instead of %p)\n",
 		      backbuffer, buffer);
 		return -EINVAL;
+		goto err;
 	}
 
 	ret = _ehci_destroy_int_queue(dev, queue);
@@ -1565,6 +1566,10 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 
 	/* everything worked out fine */
 	return result;
+	
+err:
+    _ehci_destroy_int_queue(dev, queue);  // Освобождаем очередь перед выходом
+    return result;
 }
 
 static int _ehci_lock_async(struct ehci_ctrl *ctrl, int lock)
-- 
2.30.2

[PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[ant.v.moryakov]

From: Anton Moryakov <ant.v.moryakov@gmail.com>

Ensure int_queue is properly destroyed when receiving wrong buffer
by adding error handling path. Fixes memory leak that occurred
when backbuffer validation failed.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
 drivers/usb/host/ehci-hcd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 7d5519c65a9..765d8b327ee 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1557,6 +1557,7 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 		debug("got wrong buffer back (%p instead of %p)\n",
 		      backbuffer, buffer);
 		return -EINVAL;
+		goto err;
 	}
 
 	ret = _ehci_destroy_int_queue(dev, queue);
@@ -1565,6 +1566,10 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 
 	/* everything worked out fine */
 	return result;
+	
+err:
+    _ehci_destroy_int_queue(dev, queue);
+    return result;
 }
 
 static int _ehci_lock_async(struct ehci_ctrl *ctrl, int lock)
-- 
2.30.2

Re: [PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Fri, May 16, 2025 at 08:54:34PM +0300, ant.v.moryakov@gmail.com wrote:

> From: Anton Moryakov <ant.v.moryakov@gmail.com>
> 
> Ensure int_queue is properly destroyed when receiving wrong buffer
> by adding error handling path. Fixes memory leak that occurred
> when backbuffer validation failed.
> 
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> ---
>  drivers/usb/host/ehci-hcd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 7d5519c65a9..765d8b327ee 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -1557,6 +1557,7 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
>  		debug("got wrong buffer back (%p instead of %p)\n",
>  		      backbuffer, buffer);
>  		return -EINVAL;
> +		goto err;

A goto after return cannot be right, and should have had some compiler
warnings too.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[Anton Moryakov]

Thanks Tom — you're right, that `goto` after `return` is unreachable. I'll
fix that and resend with the corrected flow.

пт, 16 мая 2025 г. в 21:31, Tom Rini <trini@konsulko.com>:

> On Fri, May 16, 2025 at 08:54:34PM +0300, ant.v.moryakov@gmail.com wrote:
>
> > From: Anton Moryakov <ant.v.moryakov@gmail.com>
> >
> > Ensure int_queue is properly destroyed when receiving wrong buffer
> > by adding error handling path. Fixes memory leak that occurred
> > when backbuffer validation failed.
> >
> > Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> > ---
> >  drivers/usb/host/ehci-hcd.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index 7d5519c65a9..765d8b327ee 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -1557,6 +1557,7 @@ static int _ehci_submit_int_msg(struct usb_device
> *dev, unsigned long pipe,
> >               debug("got wrong buffer back (%p instead of %p)\n",
> >                     backbuffer, buffer);
> >               return -EINVAL;
> > +             goto err;
>
> A goto after return cannot be right, and should have had some compiler
> warnings too.
>
> --
> Tom
>

Re: [PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

On Fri, May 16, 2025 at 09:57:11PM +0300, Anton Moryakov wrote:

> Thanks Tom — you're right, that `goto` after `return` is unreachable. I'll
> fix that and resend with the corrected flow.

OK. Please make sure that for all of the patches you're posting, you're
not adding warnings. You can enable CONFIG_WERROR to make the easier.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[ant.v.moryakov]

From: Anton Moryakov <ant.v.moryakov@gmail.com>

Ensure int_queue is properly destroyed when receiving wrong buffer
by adding error handling path. Fixes memory leak that occurred
when backbuffer validation failed.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
 drivers/usb/host/ehci-hcd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 7d5519c65a9..765d8b327ee 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1557,6 +1557,7 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 		debug("got wrong buffer back (%p instead of %p)\n",
 		      backbuffer, buffer);
 		result = -EINVAL;
+		goto err;
 	}
 
 	ret = _ehci_destroy_int_queue(dev, queue);
@@ -1565,6 +1566,10 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 
 	/* everything worked out fine */
 	return result;
+	
+err:
+    _ehci_destroy_int_queue(dev, queue);  // Освобождаем очередь перед выходом
+    return result;
 }
 
 static int _ehci_lock_async(struct ehci_ctrl *ctrl, int lock)
-- 
2.30.2

Re: [PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[Fabio Estevam]

On Fri, May 16, 2025 at 4:43 PM <ant.v.moryakov@gmail.com> wrote:

> @@ -1557,6 +1557,7 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
>                 debug("got wrong buffer back (%p instead of %p)\n",
>                       backbuffer, buffer);
>                 result = -EINVAL;
> +               goto err;

Was this supposed to be a v2? The error Tim pointed out is still here.

     ret = _ehci_destroy_int_queue(dev, queue);
> @@ -1565,6 +1566,10 @@ static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe,
>
>         /* everything worked out fine */
>         return result;
> +
> +err:
> +    _ehci_destroy_int_queue(dev, queue);  // Освобождаем очередь перед выходом

Please remove the comment.

Re: [PATCH] drivers: usb: host: fix prevent memory leak in interrupt message submission

[Anton Moryakov]

Hi Fabio,

Thanks for the review. Yes, this is v2 — the issue Tom pointed out
(unreachable `goto` after `return`) was fixed by replacing it with `result
= -EINVAL; goto err;`.

In v3 I’ve now removed the leftover comment. No other functional changes.

Best regards,
Anton

пт, 16 мая 2025 г. в 22:59, Fabio Estevam <festevam@gmail.com>:

> On Fri, May 16, 2025 at 4:43 PM <ant.v.moryakov@gmail.com> wrote:
>
> > @@ -1557,6 +1557,7 @@ static int _ehci_submit_int_msg(struct usb_device
> *dev, unsigned long pipe,
> >                 debug("got wrong buffer back (%p instead of %p)\n",
> >                       backbuffer, buffer);
> >                 result = -EINVAL;
> > +               goto err;
>
> Was this supposed to be a v2? The error Tim pointed out is still here.
>
>      ret = _ehci_destroy_int_queue(dev, queue);
> > @@ -1565,6 +1566,10 @@ static int _ehci_submit_int_msg(struct usb_device
> *dev, unsigned long pipe,
> >
> >         /* everything worked out fine */
> >         return result;
> > +
> > +err:
> > +    _ehci_destroy_int_queue(dev, queue);  // Освобождаем очередь перед
> выходом
>
> Please remove the comment.
>