FAILED: patch "[PATCH] usb: xhci: Handle error condition in xhci_stop_device()" failed to apply to 4.9-stable tree

FAILED: patch "[PATCH] usb: xhci: Handle error condition in xhci_stop_device()" failed to apply to 4.9-stable tree

[gregkh]

The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From b3207c65dfafae27e7c492cb9188c0dc0eeaf3fd Mon Sep 17 00:00:00 2001
From: Mayank Rana <mrana@codeaurora.org>
Date: Fri, 6 Oct 2017 17:45:30 +0300
Subject: [PATCH] usb: xhci: Handle error condition in xhci_stop_device()

xhci_stop_device() calls xhci_queue_stop_endpoint() multiple times
without checking the return value. xhci_queue_stop_endpoint() can
return error if the HC is already halted or unable to queue commands.
This can cause a deadlock condition as xhci_stop_device() would
end up waiting indefinitely for a completion for the command that
didn't get queued. Fix this by checking the return value and bailing
out of xhci_stop_device() in case of error. This patch happens to fix
potential memory leaks of the allocated command structures as well.

Fixes: c311e391a7ef ("xhci: rework command timeout and cancellation,")
Cc: <stable@vger.kernel.org>
Signed-off-by: Mayank Rana <mrana@codeaurora.org>
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index da9158f171cb..a2336deb5e36 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -420,14 +420,25 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
 						     GFP_NOWAIT);
 			if (!command) {
 				spin_unlock_irqrestore(&xhci->lock, flags);
-				xhci_free_command(xhci, cmd);
-				return -ENOMEM;
+				ret = -ENOMEM;
+				goto cmd_cleanup;
+			}
+
+			ret = xhci_queue_stop_endpoint(xhci, command, slot_id,
+						       i, suspend);
+			if (ret) {
+				spin_unlock_irqrestore(&xhci->lock, flags);
+				xhci_free_command(xhci, command);
+				goto cmd_cleanup;
 			}
-			xhci_queue_stop_endpoint(xhci, command, slot_id, i,
-						 suspend);
 		}
 	}
-	xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
+	ret = xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
+	if (ret) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		goto cmd_cleanup;
+	}
+
 	xhci_ring_cmd_db(xhci);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
@@ -439,6 +450,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
 		xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n");
 		ret = -ETIME;
 	}
+
+cmd_cleanup:
 	xhci_free_command(xhci, cmd);
 	return ret;
 }

Re: FAILED: patch "[PATCH] usb: xhci: Handle error condition in xhci_stop_device()" failed to apply to 4.9-stable tree

[Jack Pham]

Hi Greg,

On Mon, Oct 23, 2017 at 02:52:50PM +0200, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 4.9-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.

It appears to be a trivial conflict. See below.

> ------------------ original commit in Linus's tree ------------------
> 
> From b3207c65dfafae27e7c492cb9188c0dc0eeaf3fd Mon Sep 17 00:00:00 2001
> From: Mayank Rana <mrana@codeaurora.org>
> Date: Fri, 6 Oct 2017 17:45:30 +0300
> Subject: [PATCH] usb: xhci: Handle error condition in xhci_stop_device()
> 
> xhci_stop_device() calls xhci_queue_stop_endpoint() multiple times
> without checking the return value. xhci_queue_stop_endpoint() can
> return error if the HC is already halted or unable to queue commands.
> This can cause a deadlock condition as xhci_stop_device() would
> end up waiting indefinitely for a completion for the command that
> didn't get queued. Fix this by checking the return value and bailing
> out of xhci_stop_device() in case of error. This patch happens to fix
> potential memory leaks of the allocated command structures as well.
> 
> Fixes: c311e391a7ef ("xhci: rework command timeout and cancellation,")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Mayank Rana <mrana@codeaurora.org>
> Signed-off-by: Jack Pham <jackp@codeaurora.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index da9158f171cb..a2336deb5e36 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -420,14 +420,25 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
>  						     GFP_NOWAIT);
>  			if (!command) {
>  				spin_unlock_irqrestore(&xhci->lock, flags);
> -				xhci_free_command(xhci, cmd);
> -				return -ENOMEM;

The conflict appears to be here. There was an extra blank line after
this that had been removed by another patch in 4.13. So when applying to
the older stable trees which still has the blank line I guess git
couldn't resolve it.

Is this enough info for you to fix up or would you need me to send a
revised backport patch? If I send, would I need to add my S-o-b again
after the other tags?
 
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: FAILED: patch "[PATCH] usb: xhci: Handle error condition in xhci_stop_device()" failed to apply to 4.9-stable tree

[Greg KH]

On Mon, Oct 23, 2017 at 06:13:50PM -0700, Jack Pham wrote:
> Hi Greg,
> 
> On Mon, Oct 23, 2017 at 02:52:50PM +0200, gregkh@linuxfoundation.org wrote:
> > 
> > The patch below does not apply to the 4.9-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> 
> It appears to be a trivial conflict. See below.
> 
> > ------------------ original commit in Linus's tree ------------------
> > 
> > From b3207c65dfafae27e7c492cb9188c0dc0eeaf3fd Mon Sep 17 00:00:00 2001
> > From: Mayank Rana <mrana@codeaurora.org>
> > Date: Fri, 6 Oct 2017 17:45:30 +0300
> > Subject: [PATCH] usb: xhci: Handle error condition in xhci_stop_device()
> > 
> > xhci_stop_device() calls xhci_queue_stop_endpoint() multiple times
> > without checking the return value. xhci_queue_stop_endpoint() can
> > return error if the HC is already halted or unable to queue commands.
> > This can cause a deadlock condition as xhci_stop_device() would
> > end up waiting indefinitely for a completion for the command that
> > didn't get queued. Fix this by checking the return value and bailing
> > out of xhci_stop_device() in case of error. This patch happens to fix
> > potential memory leaks of the allocated command structures as well.
> > 
> > Fixes: c311e391a7ef ("xhci: rework command timeout and cancellation,")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Mayank Rana <mrana@codeaurora.org>
> > Signed-off-by: Jack Pham <jackp@codeaurora.org>
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> > index da9158f171cb..a2336deb5e36 100644
> > --- a/drivers/usb/host/xhci-hub.c
> > +++ b/drivers/usb/host/xhci-hub.c
> > @@ -420,14 +420,25 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
> >  						     GFP_NOWAIT);
> >  			if (!command) {
> >  				spin_unlock_irqrestore(&xhci->lock, flags);
> > -				xhci_free_command(xhci, cmd);
> > -				return -ENOMEM;
> 
> The conflict appears to be here. There was an extra blank line after
> this that had been removed by another patch in 4.13. So when applying to
> the older stable trees which still has the blank line I guess git
> couldn't resolve it.
> 
> Is this enough info for you to fix up or would you need me to send a
> revised backport patch? If I send, would I need to add my S-o-b again
> after the other tags?

I need a new patch please, that makes it the easiest for me to ensure I
got it right :)

And if you want to add a new s-o-b, that's fine as well, but not needed.

thanks,

greg k-h

[PATCH] usb: xhci: Handle error condition in xhci_stop_device()

[Jack Pham]

From: Mayank Rana <mrana@codeaurora.org>

commit b3207c65dfafae27e7c492cb9188c0dc0eeaf3fd upstream.

xhci_stop_device() calls xhci_queue_stop_endpoint() multiple times
without checking the return value. xhci_queue_stop_endpoint() can
return error if the HC is already halted or unable to queue commands.
This can cause a deadlock condition as xhci_stop_device() would
end up waiting indefinitely for a completion for the command that
didn't get queued. Fix this by checking the return value and bailing
out of xhci_stop_device() in case of error. This patch happens to fix
potential memory leaks of the allocated command structures as well.

Fixes: c311e391a7ef ("xhci: rework command timeout and cancellation,")
Signed-off-by: Mayank Rana <mrana@codeaurora.org>
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Hi Greg,

I tested that this applies cleanly to 3.18.y, 4.4.y and 4.9.y.

Thanks,
jack

 drivers/usb/host/xhci-hub.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 4a02c5c..0722f75 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -412,15 +412,25 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
 						     GFP_NOWAIT);
 			if (!command) {
 				spin_unlock_irqrestore(&xhci->lock, flags);
-				xhci_free_command(xhci, cmd);
-				return -ENOMEM;
+				ret = -ENOMEM;
+				goto cmd_cleanup;
+			}
 
+			ret = xhci_queue_stop_endpoint(xhci, command, slot_id,
+						       i, suspend);
+			if (ret) {
+				spin_unlock_irqrestore(&xhci->lock, flags);
+				xhci_free_command(xhci, command);
+				goto cmd_cleanup;
 			}
-			xhci_queue_stop_endpoint(xhci, command, slot_id, i,
-						 suspend);
 		}
 	}
-	xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
+	ret = xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
+	if (ret) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		goto cmd_cleanup;
+	}
+
 	xhci_ring_cmd_db(xhci);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
@@ -431,6 +441,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
 		xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n");
 		ret = -ETIME;
 	}
+
+cmd_cleanup:
 	xhci_free_command(xhci, cmd);
 	return ret;
 }
-- 
2.9.1.200.gb1ec08f

Re: [PATCH] usb: xhci: Handle error condition in xhci_stop_device()

[Greg KH]

On Tue, Oct 24, 2017 at 09:16:19AM -0700, Jack Pham wrote:
> From: Mayank Rana <mrana@codeaurora.org>
> 
> commit b3207c65dfafae27e7c492cb9188c0dc0eeaf3fd upstream.
> 
> xhci_stop_device() calls xhci_queue_stop_endpoint() multiple times
> without checking the return value. xhci_queue_stop_endpoint() can
> return error if the HC is already halted or unable to queue commands.
> This can cause a deadlock condition as xhci_stop_device() would
> end up waiting indefinitely for a completion for the command that
> didn't get queued. Fix this by checking the return value and bailing
> out of xhci_stop_device() in case of error. This patch happens to fix
> potential memory leaks of the allocated command structures as well.
> 
> Fixes: c311e391a7ef ("xhci: rework command timeout and cancellation,")
> Signed-off-by: Mayank Rana <mrana@codeaurora.org>
> Signed-off-by: Jack Pham <jackp@codeaurora.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Hi Greg,
> 
> I tested that this applies cleanly to 3.18.y, 4.4.y and 4.9.y.

Thanks for the patch, now queued up.

greg k-h