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

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

[Lu Baolu]

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

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

[Greg KH]

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

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

[Lu Baolu]

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

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

[Greg KH]

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

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

[Lu Baolu]

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