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.sharp@xxxxxxxxxxxxxxx> Cc: stable@xxxxxxxxxxxxxxx Cc: Xenia Ragiadakou <burzalodowa@xxxxxxxxx> --- 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html