If a control transfer to a parent hub to disable the U1/U2 timeout
fails when usb_reset_and_verify_device() calls into
usb_unlocked_disable_lpm(), the USB core will attempt to re-enable LPM.
That will cause an oops in usb_enable_link_state:

[ 1466.722795] usb 2-2.3: Failed to set U1 timeout to 0x0,error code -71
[ 1466.722817] BUG: unable to handle kernel NULL pointer dereference at 
0000000000000010
[ 1466.722888] IP: [<ffffffff814a74cd>] usb_enable_link_state+0x2d/0x2f0
[ 1466.722969] PGD 0
[ 1466.722990] Oops: 0000 [#1] SMP
[ 1466.723026] Modules linked in: pegasus mii ses enclosure usb_storage cuse 
dm_crypt uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core videodev 
x86_pkg_temp_thermal coretemp ghash_clmu
lni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd 
arc4 iwldvm mac80211 snd_hda_codec_hdmi microcode snd_hda_codec_realtek 
snd_usb_audio joydev thinkpad_acpi snd_hda_in
tel iwlwifi snd_usbmidi_lib nvram snd_hda_codec snd_seq_midi snd_seq_midi_event 
psmouse snd_rawmidi snd_hwdep serio_raw snd_pcm cfg80211 snd_seq bnep rfcomm 
bluetooth snd_page_alloc snd_seq_devi
ce ehci_pci snd_timer ehci_hcd lpc_ich snd soundcore tpm_tis binfmt_misc btrfs 
libcrc32c xor raid6_pq hid_generic usbhid hid i915 i2c_algo_bit drm_kms_helper 
sdhci_pci e1000e drm sdhci ahci liba
hci xhci_hcd ptp pps_core video
[ 1466.723790] CPU: 3 PID: 3516 Comm: usb-storage Tainted: G        W    
3.13.0-rc1+ #150
[ 1466.723861] Hardware name: LENOVO 2325AP7/2325AP7, BIOS G2ET82WW (2.02 ) 
09/11/2012
[ 1466.723926] task: ffff8800b15ac120 ti: ffff880085e52000 task.ti: 
ffff880085e52000
[ 1466.723980] RIP: 0010:[<ffffffff814a74cd>]  [<ffffffff814a74cd>] 
usb_enable_link_state+0x2d/0x2f0
[ 1466.724054] RSP: 0018:ffff880085e53b98  EFLAGS: 00010246
[ 1466.724093] RAX: 0000000000000000 RBX: ffff88009eaed800 RCX: 0000000000000001
[ 1466.724144] RDX: 0000000000000001 RSI: ffff88009eaed800 RDI: ffff880031be6000
[ 1466.724199] RBP: ffff880085e53be8 R08: 0000000000000002 R09: 0000000000000001
[ 1466.724249] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
[ 1466.724305] R13: ffff880082f86000 R14: ffff880031be6000 R15: 0000000000000003
[ 1466.724356] FS:  0000000000000000(0000) GS:ffff88011e2c0000(0000) 
knlGS:0000000000000000
[ 1466.724418] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1466.724459] CR2: 0000000000000010 CR3: 00000000c628d000 CR4: 00000000001407e0
[ 1466.724509] Stack:
[ 1466.724525]  0000000000000000 00000000ffffffb9 0000000000000000 
ffff880085e53be0
[ 1466.724589]  ffffffff814a677b ffff88009eaed800 ffff880031be6000 
ffff880082f86000
[ 1466.724655]  0000000000000003 0000000000000003 ffff880085e53c08 
ffffffff814a7811
[ 1466.724754] Call Trace:
[ 1466.724782]  [<ffffffff814a677b>] ? usb_set_lpm_timeout+0x12b/0x140
[ 1466.724830]  [<ffffffff814a7811>] usb_enable_lpm+0x81/0xa0
[ 1466.724873]  [<ffffffff814a7918>] usb_disable_lpm+0xa8/0xc0
[ 1466.724922]  [<ffffffff814a795e>] usb_unlocked_disable_lpm+0x2e/0x50
[ 1466.724971]  [<ffffffff814aafc0>] usb_reset_and_verify_device+0xc0/0x770
[ 1466.725022]  [<ffffffff810981fd>] ? trace_hardirqs_on+0xd/0x10
[ 1466.725075]  [<ffffffffa07333ac>] ? usb_stor_pre_reset+0x1c/0x20 
[usb_storage]
[ 1466.725129]  [<ffffffff814369cc>] ? __pm_runtime_resume+0x5c/0x90
[ 1466.725196]  [<ffffffff814abd18>] usb_reset_device+0xe8/0x1d0
[ 1466.725243]  [<ffffffffa0732d9c>] usb_stor_port_reset+0x6c/0x80 [usb_storage]
[ 1466.725294]  [<ffffffffa0732e3e>] usb_stor_invoke_transport+0x8e/0x550 
[usb_storage]
[ 1466.725372]  [<ffffffffa0733be6>] ? usb_stor_control_thread+0x96/0x290 
[usb_storage]
[ 1466.725447]  [<ffffffffa0731b7e>] usb_stor_transparent_scsi_command+0xe/0x10 
[usb_storage]
[ 1466.725509]  [<ffffffffa0733cc3>] usb_stor_control_thread+0x173/0x290 
[usb_storage]
[ 1466.725576]  [<ffffffffa0733b50>] ? fill_inquiry_response+0x20/0x20 
[usb_storage]
[ 1466.725666]  [<ffffffffa0733b50>] ? fill_inquiry_response+0x20/0x20 
[usb_storage]
[ 1466.725695]  [<ffffffff8106dc8c>] kthread+0xfc/0x120
[ 1466.725748]  [<ffffffff8106db90>] ? kthread_create_on_node+0x230/0x230
[ 1466.725834]  [<ffffffff816695ec>] ret_from_fork+0x7c/0xb0
[ 1466.725878]  [<ffffffff8106db90>] ? kthread_create_on_node+0x230/0x230
[ 1466.725930] Code: 44 00 00 55 48 89 e5 41 57 41 56 49 89 fe 41 55 41 54 41 
89 d4 53 48 89 f3 48 83 ec 28 48 8b 86 98 04 00 00 41 83 fc 01 0f 94 c1 <48> 8b 
40 10 0f b7 50 08 74 79 41 83 fc 02 40 0f 94 c6 75 17 66
[ 1466.729716] hub 2-0:1.0: port 2 not warm reset yet, waiting 200ms
[ 1466.734229] RIP  [<ffffffff814a74cd>] usb_enable_link_state+0x2d/0x2f0
[ 1466.737086]  RSP <ffff880085e53b98>
[ 1466.739767] CR2: 0000000000000010
[ 1466.753046] ---[ end trace 258408fcefe55312 ]---

        /* If the device says it doesn't have *any* exit latency to come out of
         * U1 or U2, it's probably lying.  Assume it doesn't implement that link
         * state.
         */
        if ((state == USB3_LPM_U1 && u1_mel == 0) ||
 ffffffff814a74c6:      41 83 fc 01             cmp    $0x1,%r12d  |  %r12 => 1
 ffffffff814a74ca:      0f 94 c1                sete   %cl
  */
 static void usb_enable_link_state(struct usb_hcd *hcd, struct usb_device *udev,
                enum usb3_link_state state)
 {
        int timeout, ret;
        __u8 u1_mel = udev->bos->ss_cap->bU1devExitLat;
*ffffffff814a74cd:      48 8b 40 10             mov    0x10(%rax),%rax |  %eax 
= 0 <--- faulting instruction
        __le16 u2_mel = udev->bos->ss_cap->bU2DevExitLat;
 ffffffff814a74d1:      0f b7 50 08             movzwl 0x8(%rax),%edx

        /* If the device says it doesn't have *any* exit latency to come out of
         * U1 or U2, it's probably lying.  Assume it doesn't implement that link
         * state.
         */
        if ((state == USB3_LPM_U1 && u1_mel == 0) ||
 ffffffff814a74d5:      74 79                   je     ffffffff814a7550 
<usb_enable_link_state+0xb0>
                        (state == USB3_LPM_U2 && u2_mel == 0))
 ffffffff814a74d7:      41 83 fc 02             cmp    $0x2,%r12d

        /* If the device says it doesn't have *any* exit latency to come out of

This is caused by commit e3376d6c87eea09bd65ece6073f6e5d47aa560a3
"usbcore: compare and release one bos descriptor in
usb_reset_and_verify_device()".  That commit saves the old BOS
descriptor in a local variable, and sets udev->bos to NULL before
calling into usb_unlocked_disable_lpm().

Normally, that's fine, because the USB core and xHCI driver disable LPM
paths don't dereference the bos descriptor.  However, when disabling LPM
fails and the USB core re-enables LPM, usb_enable_lpm() calls into
usb_enable_link_state(), which attempts to dereference the bos
descriptor.

Fix this by moving the code to set udev->bos to NULL after LPM has been
disabled.  Make sure to change the error paths to avoid a double free of
the BOS descriptor.  Also, change usb_enable_lpm() to return if either
the BOS descriptor or the SuperSpeed Extended Capabilities BOS
descriptor is NULL.  It's possible (although not likely, since it would
be out-of-spec) that we could have a USB 3.0 device without a BOS
descriptor.

This patch should be backported to kernels as old as 3.12, that contain
the commit e3376d6c87eea09bd65ece6073f6e5d47aa560a3 ""usbcore: compare
and release one bos descriptor in usb_reset_and_verify_device()".

Signed-off-by: Sarah Sharp <sarah.a.sh...@linux.intel.com>
Cc: sta...@vger.kernel.org
Cc: Xenia Ragiadakou <burzalod...@gmail.com>
---
 drivers/usb/core/hub.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bd9dc3504b51..651aed3b870f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3788,6 +3788,9 @@ void usb_enable_lpm(struct usb_device *udev)
                        !udev->lpm_capable)
                return;
 
+       if (!udev->bos || !udev->bos->ss_cap)
+               return;
+
        udev->lpm_disable_count--;
        hcd = bus_to_hcd(udev->bus);
        /* Double check that we can both enable and disable LPM.
@@ -5102,7 +5105,7 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
        struct usb_hub                  *parent_hub;
        struct usb_hcd                  *hcd = bus_to_hcd(udev->bus);
        struct usb_device_descriptor    descriptor = udev->descriptor;
-       struct usb_host_bos             *bos;
+       struct usb_host_bos             *bos = NULL;
        int                             i, ret = 0;
        int                             port1 = udev->portnum;
 
@@ -5126,9 +5129,6 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
        if (udev->usb2_hw_lpm_enabled == 1)
                usb_set_usb2_hardware_lpm(udev, 0);
 
-       bos = udev->bos;
-       udev->bos = NULL;
-
        /* Disable LPM and LTM while we reset the device and reinstall the alt
         * settings.  Device-initiated LPM settings, and system exit latency
         * settings are cleared when the device is reset, so we have to set
@@ -5146,6 +5146,9 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
                goto re_enumerate;
        }
 
+       bos = udev->bos;
+       udev->bos = NULL;
+
        set_bit(port1, parent_hub->busy_bits);
        for (i = 0; i < SET_CONFIG_TRIES; ++i) {
 
@@ -5159,13 +5162,13 @@ static int usb_reset_and_verify_device(struct 
usb_device *udev)
        clear_bit(port1, parent_hub->busy_bits);
 
        if (ret < 0)
-               goto re_enumerate;
+               goto free_bos;
 
        /* Device might have changed firmware (DFU or similar) */
        if (descriptors_changed(udev, &descriptor, bos)) {
                dev_info(&udev->dev, "device firmware changed\n");
                udev->descriptor = descriptor;  /* for disconnect() calls */
-               goto re_enumerate;
+               goto free_bos;
        }
 
        /* Restore the device's previous configuration */
@@ -5179,7 +5182,7 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
                                "Busted HC?  Not enough HCD resources for "
                                "old configuration.\n");
                mutex_unlock(hcd->bandwidth_mutex);
-               goto re_enumerate;
+               goto free_bos;
        }
        ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
                        USB_REQ_SET_CONFIGURATION, 0,
@@ -5190,7 +5193,7 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
                        "can't restore configuration #%d (error=%d)\n",
                        udev->actconfig->desc.bConfigurationValue, ret);
                mutex_unlock(hcd->bandwidth_mutex);
-               goto re_enumerate;
+               goto free_bos;
        }
        mutex_unlock(hcd->bandwidth_mutex);
        usb_set_device_state(udev, USB_STATE_CONFIGURED);
@@ -5227,7 +5230,7 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
                                desc->bInterfaceNumber,
                                desc->bAlternateSetting,
                                ret);
-                       goto re_enumerate;
+                       goto free_bos;
                }
        }
 
@@ -5240,11 +5243,12 @@ done:
        udev->bos = bos;
        return 0;
 
+free_bos:
+       usb_release_bos_descriptor(udev);
+       udev->bos = bos;
 re_enumerate:
        /* LPM state doesn't matter when we're about to destroy the device. */
        hub_port_logical_disconnect(parent_hub, port1);
-       usb_release_bos_descriptor(udev);
-       udev->bos = bos;
        return -ENODEV;
 }
 
-- 
1.8.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to