Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1478703 > unrolled thread

[PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device()

Started byLu Baolu <baolu.lu@linux.intel.com>
First post2016-09-08 02:50 +0200
Last post2016-09-08 11:10 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device() Lu Baolu <baolu.lu@linux.intel.com> - 2016-09-08 02:50 +0200
    Re: [PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device() Greg KH <gregkh@linuxfoundation.org> - 2016-09-08 08:40 +0200
      Re: [PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device() Lu Baolu <baolu.lu@linux.intel.com> - 2016-09-08 09:30 +0200
        Re: [PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device() Greg KH <gregkh@linuxfoundation.org> - 2016-09-08 11:00 +0200
          Re: [PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device() Lu Baolu <baolu.lu@linux.intel.com> - 2016-09-08 11:10 +0200

#1478703 — [PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device()

FromLu Baolu <baolu.lu@linux.intel.com>
Date2016-09-08 02:50 +0200
Subject[PATCH v2 1/1] usb: xhci: fix return value of xhci_setup_device()
Message-ID<seVHs-7bz-5@gated-at.bofh.it>
xhci_setup_device() should return failure with correct error number
when xhci host has died, removed or halted.

Cc: stable@vger.kernel.org # 4.3+
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 v1->v2:
 - fix email mismatch issue

 drivers/usb/host/xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 01d96c9..3e66e73 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3785,8 +3785,10 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 
 	mutex_lock(&xhci->mutex);
 
-	if (xhci->xhc_state)	/* dying, removing or halted */
+	if (xhci->xhc_state) {	/* dying, removing or halted */
+		ret = -ESHUTDOWN;
 		goto out;
+	}
 
 	if (!udev->slot_id) {
 		xhci_dbg_trace(xhci, trace_xhci_dbg_address,
-- 
2.1.4

[toc] | [next] | [standalone]


#1478800

FromGreg KH <gregkh@linuxfoundation.org>
Date2016-09-08 08:40 +0200
Message-ID<sf1aa-2fT-13@gated-at.bofh.it>
In reply to#1478703
On Thu, Sep 08, 2016 at 08:41:02AM +0800, Lu Baolu wrote:
> xhci_setup_device() should return failure with correct error number
> when xhci host has died, removed or halted.
> 
> Cc: stable@vger.kernel.org # 4.3+

Why is this a stable kernel issue?  What bug does it fix that affects
users?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]


#1478859

FromLu Baolu <baolu.lu@linux.intel.com>
Date2016-09-08 09:30 +0200
Message-ID<sf1Wx-2Ly-1@gated-at.bofh.it>
In reply to#1478800
Hi Greg,

On 09/08/2016 02:38 PM, Greg KH wrote:
> On Thu, Sep 08, 2016 at 08:41:02AM +0800, Lu Baolu wrote:
>> xhci_setup_device() should return failure with correct error number
>> when xhci host has died, removed or halted.
>>
>> Cc: stable@vger.kernel.org # 4.3+
> Why is this a stable kernel issue?  What bug does it fix that affects
> users?

During usb device enumeration, if xhci host is not accessible (died,
removed or halted), the hc_driver->address_device() should return
a corresponding error code to usb core. But current xhci driver just
returns success. This will mislead usb core to continue enumeration:
reading device descriptor, which will result in failure, and users will
get a misleading message like "device descriptor read/8, error -110".

Best regards,
Lu Baolu

[toc] | [prev] | [next] | [standalone]


#1478941

FromGreg KH <gregkh@linuxfoundation.org>
Date2016-09-08 11:00 +0200
Message-ID<sf3lE-3zk-25@gated-at.bofh.it>
In reply to#1478859
On Thu, Sep 08, 2016 at 03:29:25PM +0800, Lu Baolu wrote:
> Hi Greg,
> 
> On 09/08/2016 02:38 PM, Greg KH wrote:
> > On Thu, Sep 08, 2016 at 08:41:02AM +0800, Lu Baolu wrote:
> >> xhci_setup_device() should return failure with correct error number
> >> when xhci host has died, removed or halted.
> >>
> >> Cc: stable@vger.kernel.org # 4.3+
> > Why is this a stable kernel issue?  What bug does it fix that affects
> > users?
> 
> During usb device enumeration, if xhci host is not accessible (died,
> removed or halted), the hc_driver->address_device() should return
> a corresponding error code to usb core. But current xhci driver just
> returns success. This will mislead usb core to continue enumeration:
> reading device descriptor, which will result in failure, and users will
> get a misleading message like "device descriptor read/8, error -110".

Why didn't you include this in the changelog text so that it is obvious
why this patch is needed?

Don't just describe what the patch does, we can read C code, describe
_why_ it is needed.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]


#1478943

FromLu Baolu <baolu.lu@linux.intel.com>
Date2016-09-08 11:10 +0200
Message-ID<sf3vj-3RO-1@gated-at.bofh.it>
In reply to#1478941
Hi Greg,

On 09/08/2016 04:58 PM, Greg KH wrote:
> On Thu, Sep 08, 2016 at 03:29:25PM +0800, Lu Baolu wrote:
>> Hi Greg,
>>
>> On 09/08/2016 02:38 PM, Greg KH wrote:
>>> On Thu, Sep 08, 2016 at 08:41:02AM +0800, Lu Baolu wrote:
>>>> xhci_setup_device() should return failure with correct error number
>>>> when xhci host has died, removed or halted.
>>>>
>>>> Cc: stable@vger.kernel.org # 4.3+
>>> Why is this a stable kernel issue?  What bug does it fix that affects
>>> users?
>> During usb device enumeration, if xhci host is not accessible (died,
>> removed or halted), the hc_driver->address_device() should return
>> a corresponding error code to usb core. But current xhci driver just
>> returns success. This will mislead usb core to continue enumeration:
>> reading device descriptor, which will result in failure, and users will
>> get a misleading message like "device descriptor read/8, error -110".
> Why didn't you include this in the changelog text so that it is obvious
> why this patch is needed?
>
> Don't just describe what the patch does, we can read C code, describe
> _why_ it is needed.

Yes. Thank you for the guidance. I will do it in a v3 patch.

Best regards,
Lu Baolu

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web