[PATCH] usb: Fix oops on LPM disable failure.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux