linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] xhci fixes for usb-linus
@ 2019-05-22 11:33 Mathias Nyman
  2019-05-22 11:33 ` [PATCH 1/5] xhci: update bounce buffer with correct sg num Mathias Nyman
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Mathias Nyman @ 2019-05-22 11:33 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few fixes for usb-linus, including regression fix for xhci IDT support
which was added to 5.2-rc1

-Mathias

Andrey Smirnov (1):
  xhci: Convert xhci_handshake() to use readl_poll_timeout_atomic()

Carsten Schmid (1):
  usb: xhci: avoid null pointer deref when bos field is NULL

Henry Lin (1):
  xhci: update bounce buffer with correct sg num

Jia-Ju Bai (1):
  usb: xhci: Fix a potential null pointer dereference in
    xhci_debugfs_create_endpoint()

Mathias Nyman (1):
  xhci: Fix immediate data transfer if buffer is already DMA mapped

 drivers/usb/host/xhci-debugfs.c |  3 +++
 drivers/usb/host/xhci-ring.c    | 26 +++++++++++++++++++-------
 drivers/usb/host/xhci.c         | 24 +++++++++++-------------
 drivers/usb/host/xhci.h         |  3 ++-
 4 files changed, 35 insertions(+), 21 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/5] xhci: update bounce buffer with correct sg num
  2019-05-22 11:33 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
@ 2019-05-22 11:33 ` Mathias Nyman
  2019-05-22 11:33 ` [PATCH 2/5] usb: xhci: Fix a potential null pointer dereference in xhci_debugfs_create_endpoint() Mathias Nyman
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Mathias Nyman @ 2019-05-22 11:33 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Henry Lin, # v4 . 8+, Mathias Nyman

From: Henry Lin <henryl@nvidia.com>

This change fixes a data corruption issue occurred on USB hard disk for
the case that bounce buffer is used during transferring data.

While updating data between sg list and bounce buffer, current
implementation passes mapped sg number (urb->num_mapped_sgs) to
sg_pcopy_from_buffer() and sg_pcopy_to_buffer(). This causes data
not get copied if target buffer is located in the elements after
mapped sg elements. This change passes sg number for full list to
fix issue.

Besides, for copying data from bounce buffer, calling dma_unmap_single()
on the bounce buffer before copying data to sg list can avoid cache issue.

Fixes: f9c589e142d0 ("xhci: TD-fragment, align the unsplittable case with a bounce buffer")
Cc: <stable@vger.kernel.org> # v4.8+
Signed-off-by: Henry Lin <henryl@nvidia.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index fed3385..ef7c869 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -656,6 +656,7 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
 	struct device *dev = xhci_to_hcd(xhci)->self.controller;
 	struct xhci_segment *seg = td->bounce_seg;
 	struct urb *urb = td->urb;
+	size_t len;
 
 	if (!ring || !seg || !urb)
 		return;
@@ -666,11 +667,14 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
 		return;
 	}
 
-	/* for in tranfers we need to copy the data from bounce to sg */
-	sg_pcopy_from_buffer(urb->sg, urb->num_mapped_sgs, seg->bounce_buf,
-			     seg->bounce_len, seg->bounce_offs);
 	dma_unmap_single(dev, seg->bounce_dma, ring->bounce_buf_len,
 			 DMA_FROM_DEVICE);
+	/* for in tranfers we need to copy the data from bounce to sg */
+	len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs, seg->bounce_buf,
+			     seg->bounce_len, seg->bounce_offs);
+	if (len != seg->bounce_len)
+		xhci_warn(xhci, "WARN Wrong bounce buffer read length: %ld != %d\n",
+				len, seg->bounce_len);
 	seg->bounce_len = 0;
 	seg->bounce_offs = 0;
 }
@@ -3127,6 +3131,7 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len,
 	unsigned int unalign;
 	unsigned int max_pkt;
 	u32 new_buff_len;
+	size_t len;
 
 	max_pkt = usb_endpoint_maxp(&urb->ep->desc);
 	unalign = (enqd_len + *trb_buff_len) % max_pkt;
@@ -3157,8 +3162,12 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len,
 
 	/* create a max max_pkt sized bounce buffer pointed to by last trb */
 	if (usb_urb_dir_out(urb)) {
-		sg_pcopy_to_buffer(urb->sg, urb->num_mapped_sgs,
+		len = sg_pcopy_to_buffer(urb->sg, urb->num_sgs,
 				   seg->bounce_buf, new_buff_len, enqd_len);
+		if (len != seg->bounce_len)
+			xhci_warn(xhci,
+				"WARN Wrong bounce buffer write length: %ld != %d\n",
+				len, seg->bounce_len);
 		seg->bounce_dma = dma_map_single(dev, seg->bounce_buf,
 						 max_pkt, DMA_TO_DEVICE);
 	} else {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] usb: xhci: Fix a potential null pointer dereference in xhci_debugfs_create_endpoint()
  2019-05-22 11:33 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
  2019-05-22 11:33 ` [PATCH 1/5] xhci: update bounce buffer with correct sg num Mathias Nyman
@ 2019-05-22 11:33 ` Mathias Nyman
  2019-05-22 11:33 ` [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL Mathias Nyman
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Mathias Nyman @ 2019-05-22 11:33 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Jia-Ju Bai, Mathias Nyman

From: Jia-Ju Bai <baijiaju1990@gmail.com>

In xhci_debugfs_create_slot(), kzalloc() can fail and
dev->debugfs_private will be NULL.
In xhci_debugfs_create_endpoint(), dev->debugfs_private is used without
any null-pointer check, and can cause a null pointer dereference.

To fix this bug, a null-pointer check is added in
xhci_debugfs_create_endpoint().

This bug is found by a runtime fuzzing tool named FIZZER written by us.

[subjet line change change, add potential -Mathais]
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index cadc013..7ba6afc 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -440,6 +440,9 @@ void xhci_debugfs_create_endpoint(struct xhci_hcd *xhci,
 	struct xhci_ep_priv	*epriv;
 	struct xhci_slot_priv	*spriv = dev->debugfs_private;
 
+	if (!spriv)
+		return;
+
 	if (spriv->eps[ep_index])
 		return;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL
  2019-05-22 11:33 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
  2019-05-22 11:33 ` [PATCH 1/5] xhci: update bounce buffer with correct sg num Mathias Nyman
  2019-05-22 11:33 ` [PATCH 2/5] usb: xhci: Fix a potential null pointer dereference in xhci_debugfs_create_endpoint() Mathias Nyman
@ 2019-05-22 11:33 ` Mathias Nyman
       [not found]   ` <20190529131455.C7A8C217D4@mail.kernel.org>
  2019-05-22 11:34 ` [PATCH 4/5] xhci: Fix immediate data transfer if buffer is already DMA mapped Mathias Nyman
  2019-05-22 11:34 ` [PATCH 5/5] xhci: Convert xhci_handshake() to use readl_poll_timeout_atomic() Mathias Nyman
  4 siblings, 1 reply; 13+ messages in thread
From: Mathias Nyman @ 2019-05-22 11:33 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Carsten Schmid, Stable, Mathias Nyman

From: Carsten Schmid <carsten_schmid@mentor.com>

With defective USB sticks we see the following error happen:
usb 1-3: new high-speed USB device number 6 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: device descriptor read/64, error -71
usb 1-3: new high-speed USB device number 7 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: unable to get BOS descriptor set
usb 1-3: New USB device found, idVendor=0781, idProduct=5581
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
...
BUG: unable to handle kernel NULL pointer dereference at 0000000000000008

This comes from the following place:
[ 1660.215380] IP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.222092] PGD 0 P4D 0
[ 1660.224918] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 1660.425520] CPU: 1 PID: 38 Comm: kworker/1:1 Tainted: P     U  W  O    4.14.67-apl #1
[ 1660.434277] Workqueue: usb_hub_wq hub_event [usbcore]
[ 1660.439918] task: ffffa295b6ae4c80 task.stack: ffffad4580150000
[ 1660.446532] RIP: 0010:xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.453821] RSP: 0018:ffffad4580153c70 EFLAGS: 00010046
[ 1660.459655] RAX: 0000000000000000 RBX: ffffa295b4d7c000 RCX: 0000000000000002
[ 1660.467625] RDX: 0000000000000002 RSI: ffffffff984a55b2 RDI: ffffffff984a55b2
[ 1660.475586] RBP: ffffad4580153cc8 R08: 0000000000d6520a R09: 0000000000000001
[ 1660.483556] R10: ffffad4580a004a0 R11: 0000000000000286 R12: ffffa295b4d7c000
[ 1660.491525] R13: 0000000000010648 R14: ffffa295a84e1800 R15: 0000000000000000
[ 1660.499494] FS:  0000000000000000(0000) GS:ffffa295bfc80000(0000) knlGS:0000000000000000
[ 1660.508530] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1660.514947] CR2: 0000000000000008 CR3: 000000025a114000 CR4: 00000000003406a0
[ 1660.522917] Call Trace:
[ 1660.525657]  usb_set_usb2_hardware_lpm+0x3d/0x70 [usbcore]
[ 1660.531792]  usb_disable_device+0x242/0x260 [usbcore]
[ 1660.537439]  usb_disconnect+0xc1/0x2b0 [usbcore]
[ 1660.542600]  hub_event+0x596/0x18f0 [usbcore]
[ 1660.547467]  ? trace_preempt_on+0xdf/0x100
[ 1660.552040]  ? process_one_work+0x1c1/0x410
[ 1660.556708]  process_one_work+0x1d2/0x410
[ 1660.561184]  ? preempt_count_add.part.3+0x21/0x60
[ 1660.566436]  worker_thread+0x2d/0x3f0
[ 1660.570522]  kthread+0x122/0x140
[ 1660.574123]  ? process_one_work+0x410/0x410
[ 1660.578792]  ? kthread_create_on_node+0x60/0x60
[ 1660.583849]  ret_from_fork+0x3a/0x50
[ 1660.587839] Code: 00 49 89 c3 49 8b 84 24 50 16 00 00 8d 4a ff 48 8d 04 c8 48 89 ca 4c 8b 10 45 8b 6a 04 48 8b 00 48 89 45 c0 49 8b 86 80 03 00 00 <48> 8b 40 08 8b 40 03 0f 1f 44 00 00 45 85 ff 0f 84 81 01 00 00
[ 1660.608980] RIP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd] RSP: ffffad4580153c70
[ 1660.617921] CR2: 0000000000000008

Tracking this down shows that udev->bos is NULL in the following code:
(xhci.c, in xhci_set_usb2_hardware_lpm)
	field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);  <<<<<<< here

	xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
			enable ? "enable" : "disable", port_num + 1);

	if (enable) {
		/* Host supports BESL timeout instead of HIRD */
		if (udev->usb2_hw_lpm_besl_capable) {
			/* if device doesn't have a preferred BESL value use a
			 * default one which works with mixed HIRD and BESL
			 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
			 */
			if ((field & USB_BESL_SUPPORT) &&
			    (field & USB_BESL_BASELINE_VALID))
				hird = USB_GET_BESL_BASELINE(field);
			else
				hird = udev->l1_params.besl;

The failing case is when disabling LPM. So it is sufficient to avoid
access to udev->bos by moving the instruction into the "enable" clause.

Cc: Stable <stable@vger.kernel.org>
Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a9bb796..048a675 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4320,7 +4320,6 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 	pm_addr = ports[port_num]->addr + PORTPMSC;
 	pm_val = readl(pm_addr);
 	hlpm_addr = ports[port_num]->addr + PORTHLPMC;
-	field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 
 	xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
 			enable ? "enable" : "disable", port_num + 1);
@@ -4332,6 +4331,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 			 * default one which works with mixed HIRD and BESL
 			 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
 			 */
+			field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 			if ((field & USB_BESL_SUPPORT) &&
 			    (field & USB_BESL_BASELINE_VALID))
 				hird = USB_GET_BESL_BASELINE(field);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] xhci: Fix immediate data transfer if buffer is already DMA mapped
  2019-05-22 11:33 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
                   ` (2 preceding siblings ...)
  2019-05-22 11:33 ` [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL Mathias Nyman
@ 2019-05-22 11:34 ` Mathias Nyman
  2019-05-22 11:34 ` [PATCH 5/5] xhci: Convert xhci_handshake() to use readl_poll_timeout_atomic() Mathias Nyman
  4 siblings, 0 replies; 13+ messages in thread
From: Mathias Nyman @ 2019-05-22 11:34 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman, Nicolas Saenz Julienne

xhci immediate data transfer (IDT) support in 5.2-rc1 caused regression
on various Samsung Exynos boards with ASIX USB 2.0 ethernet dongle.

If the transfer buffer in the URB is already DMA mapped then IDT should
not be used. urb->transfer_dma will already contain a valid dma address,
and there is no guarantee the data in urb->transfer_buffer is valid.

The IDT support patch used urb->transfer_dma as a temporary storage,
copying data from urb->transfer_buffer into it.

Issue was solved by preventing IDT if transfer buffer is already dma
mapped, and by not using urb->transfer_dma as temporary storage.

Fixes: 33e39350ebd2 ("usb: xhci: add Immediate Data Transfer support")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
CC: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 9 ++++++---
 drivers/usb/host/xhci.h      | 3 ++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ef7c869..88392aa 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3432,11 +3432,14 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 
 	if (urb->transfer_buffer_length > 0) {
 		u32 length_field, remainder;
+		u64 addr;
 
 		if (xhci_urb_suitable_for_idt(urb)) {
-			memcpy(&urb->transfer_dma, urb->transfer_buffer,
+			memcpy(&addr, urb->transfer_buffer,
 			       urb->transfer_buffer_length);
 			field |= TRB_IDT;
+		} else {
+			addr = (u64) urb->transfer_dma;
 		}
 
 		remainder = xhci_td_remainder(xhci, 0,
@@ -3449,8 +3452,8 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 		if (setup->bRequestType & USB_DIR_IN)
 			field |= TRB_DIR_IN;
 		queue_trb(xhci, ep_ring, true,
-				lower_32_bits(urb->transfer_dma),
-				upper_32_bits(urb->transfer_dma),
+				lower_32_bits(addr),
+				upper_32_bits(addr),
 				length_field,
 				field | ep_ring->cycle_state);
 	}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index a450a99..7f8b950 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2160,7 +2160,8 @@ static inline bool xhci_urb_suitable_for_idt(struct urb *urb)
 {
 	if (!usb_endpoint_xfer_isoc(&urb->ep->desc) && usb_urb_dir_out(urb) &&
 	    usb_endpoint_maxp(&urb->ep->desc) >= TRB_IDT_MAX_SIZE &&
-	    urb->transfer_buffer_length <= TRB_IDT_MAX_SIZE)
+	    urb->transfer_buffer_length <= TRB_IDT_MAX_SIZE &&
+	    !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
 		return true;
 
 	return false;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] xhci: Convert xhci_handshake() to use readl_poll_timeout_atomic()
  2019-05-22 11:33 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
                   ` (3 preceding siblings ...)
  2019-05-22 11:34 ` [PATCH 4/5] xhci: Fix immediate data transfer if buffer is already DMA mapped Mathias Nyman
@ 2019-05-22 11:34 ` Mathias Nyman
  4 siblings, 0 replies; 13+ messages in thread
From: Mathias Nyman @ 2019-05-22 11:34 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Andrey Smirnov, stable, Mathias Nyman

From: Andrey Smirnov <andrew.smirnov@gmail.com>

Xhci_handshake() implements the algorithm already captured by
readl_poll_timeout_atomic(). Convert the former to use the latter to
avoid repetition.

Turned out this patch also fixes a bug on the AMD Stoneyridge platform
where usleep(1) sometimes takes over 10ms.
This means a 5 second timeout can easily take over 15 seconds which will
trigger the watchdog and reboot the system.

[Add info about patch fixing a bug to commit message -Mathias]
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Tested-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-by: Raul E Rangel <rrangel@chromium.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 048a675..20db378 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/pci.h>
+#include <linux/iopoll.h>
 #include <linux/irq.h>
 #include <linux/log2.h>
 #include <linux/module.h>
@@ -52,7 +53,6 @@ static bool td_on_ring(struct xhci_td *td, struct xhci_ring *ring)
 	return false;
 }
 
-/* TODO: copied from ehci-hcd.c - can this be refactored? */
 /*
  * xhci_handshake - spin reading hc until handshake completes or fails
  * @ptr: address of hc register to be read
@@ -69,18 +69,16 @@ static bool td_on_ring(struct xhci_td *td, struct xhci_ring *ring)
 int xhci_handshake(void __iomem *ptr, u32 mask, u32 done, int usec)
 {
 	u32	result;
+	int	ret;
 
-	do {
-		result = readl(ptr);
-		if (result == ~(u32)0)		/* card removed */
-			return -ENODEV;
-		result &= mask;
-		if (result == done)
-			return 0;
-		udelay(1);
-		usec--;
-	} while (usec > 0);
-	return -ETIMEDOUT;
+	ret = readl_poll_timeout_atomic(ptr, result,
+					(result & mask) == done ||
+					result == U32_MAX,
+					1, usec);
+	if (result == U32_MAX)		/* card removed */
+		return -ENODEV;
+
+	return ret;
 }
 
 /*
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* AW: [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL
       [not found]   ` <20190529131455.C7A8C217D4@mail.kernel.org>
@ 2019-06-03  7:12     ` Schmid, Carsten
  2019-06-03 15:00     ` Schmid, Carsten
  1 sibling, 0 replies; 13+ messages in thread
From: Schmid, Carsten @ 2019-06-03  7:12 UTC (permalink / raw)
  To: Sasha Levin, Mathias Nyman, gregkh; +Cc: linux-usb, Stable, stable

Hi Sasha,

i was OOO last week.
Let me check why the patch doesn't apply in the older kernel series.
I originally developed it on a 4.14.86 and ported it to 5.1.

Shouldn't be a big problem, its a "one line mover" only.

BR
Carsten


> -----Ursprüngliche Nachricht-----
> Von: Sasha Levin [mailto:sashal@kernel.org]
> Gesendet: Mittwoch, 29. Mai 2019 15:15
> An: Sasha Levin <sashal@kernel.org>; Mathias Nyman
> <mathias.nyman@linux.intel.com>; Schmid, Carsten
> <Carsten_Schmid@mentor.com>; gregkh@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; Stable <stable@vger.kernel.org>;
> stable@vger.kernel.org
> Betreff: Re: [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is
> NULL
> 
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v5.1.4, v5.0.18, v4.19.45, v4.14.121,
> v4.9.178, v4.4.180, v3.18.140.
> 
> v5.1.4: Build OK!
> v5.0.18: Build OK!
> v4.19.45: Build OK!
> v4.14.121: Failed to apply! Possible dependencies:
>     01451ad47e272 ("powerpc/powermac: Use setup_timer() helper")
>     38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c
> functions")
>     83ad1e6a1dc04 ("powerpc/oprofile: Use setup_timer() helper")
>     8d6b1bf20f61c ("powerpc/6xx: Use setup_timer() helper")
>     b1fc2839d2f92 ("drm/msm: Implement preemption for A5XX targets")
>     b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
>     cd414f3d93168 ("drm/msm: Move memptrs to msm_gpu")
>     e629cfa36ea08 ("MIPS: Lasat: Use setup_timer() helper")
>     e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")
>     eec874ce5ff1f ("drm/msm/adreno: load gpu at probe/bind time")
>     f7de15450e906 ("drm/msm: Add per-instance submit queues")
>     f97decac5f4c2 ("drm/msm: Support multiple ringbuffers")
> 
> v4.9.178: Failed to apply! Possible dependencies:
>     01451ad47e272 ("powerpc/powermac: Use setup_timer() helper")
>     38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c
> functions")
>     53460c53b7619 ("[media] au0828: Add timer to restart TS stream if no data
> arrives on bulk endpoint")
>     7c96f59e0cafe ("[media] s5p-mfc: Fix initialization of internal structures")
>     83ad1e6a1dc04 ("powerpc/oprofile: Use setup_timer() helper")
>     8d6b1bf20f61c ("powerpc/6xx: Use setup_timer() helper")
>     b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
>     cf43e6be865a5 ("block: add scalable completion tracking of requests")
>     e629cfa36ea08 ("MIPS: Lasat: Use setup_timer() helper")
>     e806402130c9c ("block: split out request-only flags into a new namespace")
>     e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")
> 
> v4.4.180: Failed to apply! Possible dependencies:
>     01451ad47e272 ("powerpc/powermac: Use setup_timer() helper")
>     37f895d7e85e7 ("NFC: pn533: Fix socket deadlock")
>     38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c
> functions")
>     53460c53b7619 ("[media] au0828: Add timer to restart TS stream if no data
> arrives on bulk endpoint")
>     7c96f59e0cafe ("[media] s5p-mfc: Fix initialization of internal structures")
>     80c1bce9aa315 ("[media] au0828: Refactoring for start_urb_transfer()")
>     83ad1e6a1dc04 ("powerpc/oprofile: Use setup_timer() helper")
>     8d6b1bf20f61c ("powerpc/6xx: Use setup_timer() helper")
>     9815c7cf22dac ("NFC: pn533: Separate physical layer from the core
> implementation")
>     b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
>     e629cfa36ea08 ("MIPS: Lasat: Use setup_timer() helper")
>     e997ebbe46fe4 ("NFC: pn533: Send ATR_REQ only if
> NFC_PROTO_NFC_DEP bit is set")
>     e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")
> 
> v3.18.140: Failed to apply! Possible dependencies:
>     0a5942c8e1480 ("NFC: Add ACPI support for NXP PN544")
>     34ac49664149d ("NFC: nci: remove current SLEEP mode management")
>     3590ebc040c9e ("NFC: logging neatening")
>     3682f49f32051 ("NFC: netlink: Add new netlink command
> NFC_CMD_ACTIVATE_TARGET")
>     37f895d7e85e7 ("NFC: pn533: Fix socket deadlock")
>     38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c
> functions")
>     53460c53b7619 ("[media] au0828: Add timer to restart TS stream if no data
> arrives on bulk endpoint")
>     5df848f37b1d2 ("NFC: pn533: fix error return code")
>     7c96f59e0cafe ("[media] s5p-mfc: Fix initialization of internal structures")
>     80c1bce9aa315 ("[media] au0828: Refactoring for start_urb_transfer()")
>     9295b5b569fc4 ("NFC: nci: Add support for different
> NCI_DEACTIVATE_TYPE")
>     96d4581f0b371 ("NFC: netlink: Add mode parameter to deactivate_target
> functions")
>     9815c7cf22dac ("NFC: pn533: Separate physical layer from the core
> implementation")
>     b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
>     d7979e130ebb0 ("NFC: NCI: Signal deactivation in Target mode")
>     e997ebbe46fe4 ("NFC: pn533: Send ATR_REQ only if
> NFC_PROTO_NFC_DEP bit is set")
>     e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")
> 
> 
> How should we proceed with this patch?
> 
> --
> Thanks,
> Sasha

^ permalink raw reply	[flat|nested] 13+ messages in thread

* AW: [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL
       [not found]   ` <20190529131455.C7A8C217D4@mail.kernel.org>
  2019-06-03  7:12     ` AW: " Schmid, Carsten
@ 2019-06-03 15:00     ` Schmid, Carsten
  2019-06-04  6:52       ` Schmid, Carsten
  1 sibling, 1 reply; 13+ messages in thread
From: Schmid, Carsten @ 2019-06-03 15:00 UTC (permalink / raw)
  To: Sasha Levin, Mathias Nyman, gregkh; +Cc: linux-usb, Stable, Stable

[-- Attachment #1: Type: text/plain, Size: 5154 bytes --]

Hi Sasha,

i have (back)ported the patch to the older kernels mentioned below
where the original patch failed.

The patch appended to this mail applies to v4.14.121, v4.9.178, v4.4.180 and v3.18.140.
Some changes within the xhci driver prevented git from finding the correct position.

Hope this helps :-)

Best regards
Carsten

________________________________________
Von: Sasha Levin <sashal@kernel.org>
Gesendet: Mittwoch, 29. Mai 2019 15:14
An: Sasha Levin; Mathias Nyman; Schmid, Carsten; gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org; Stable; stable@vger.kernel.org
Betreff: Re: [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.1.4, v5.0.18, v4.19.45, v4.14.121, v4.9.178, v4.4.180, v3.18.140.

v5.1.4: Build OK!
v5.0.18: Build OK!
v4.19.45: Build OK!
v4.14.121: Failed to apply! Possible dependencies:
    01451ad47e272 ("powerpc/powermac: Use setup_timer() helper")
    38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c functions")
    83ad1e6a1dc04 ("powerpc/oprofile: Use setup_timer() helper")
    8d6b1bf20f61c ("powerpc/6xx: Use setup_timer() helper")
    b1fc2839d2f92 ("drm/msm: Implement preemption for A5XX targets")
    b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
    cd414f3d93168 ("drm/msm: Move memptrs to msm_gpu")
    e629cfa36ea08 ("MIPS: Lasat: Use setup_timer() helper")
    e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")
    eec874ce5ff1f ("drm/msm/adreno: load gpu at probe/bind time")
    f7de15450e906 ("drm/msm: Add per-instance submit queues")
    f97decac5f4c2 ("drm/msm: Support multiple ringbuffers")

v4.9.178: Failed to apply! Possible dependencies:
    01451ad47e272 ("powerpc/powermac: Use setup_timer() helper")
    38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c functions")
    53460c53b7619 ("[media] au0828: Add timer to restart TS stream if no data arrives on bulk endpoint")
    7c96f59e0cafe ("[media] s5p-mfc: Fix initialization of internal structures")
    83ad1e6a1dc04 ("powerpc/oprofile: Use setup_timer() helper")
    8d6b1bf20f61c ("powerpc/6xx: Use setup_timer() helper")
    b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
    cf43e6be865a5 ("block: add scalable completion tracking of requests")
    e629cfa36ea08 ("MIPS: Lasat: Use setup_timer() helper")
    e806402130c9c ("block: split out request-only flags into a new namespace")
    e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")

v4.4.180: Failed to apply! Possible dependencies:
    01451ad47e272 ("powerpc/powermac: Use setup_timer() helper")
    37f895d7e85e7 ("NFC: pn533: Fix socket deadlock")
    38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c functions")
    53460c53b7619 ("[media] au0828: Add timer to restart TS stream if no data arrives on bulk endpoint")
    7c96f59e0cafe ("[media] s5p-mfc: Fix initialization of internal structures")
    80c1bce9aa315 ("[media] au0828: Refactoring for start_urb_transfer()")
    83ad1e6a1dc04 ("powerpc/oprofile: Use setup_timer() helper")
    8d6b1bf20f61c ("powerpc/6xx: Use setup_timer() helper")
    9815c7cf22dac ("NFC: pn533: Separate physical layer from the core implementation")
    b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
    e629cfa36ea08 ("MIPS: Lasat: Use setup_timer() helper")
    e997ebbe46fe4 ("NFC: pn533: Send ATR_REQ only if NFC_PROTO_NFC_DEP bit is set")
    e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")

v3.18.140: Failed to apply! Possible dependencies:
    0a5942c8e1480 ("NFC: Add ACPI support for NXP PN544")
    34ac49664149d ("NFC: nci: remove current SLEEP mode management")
    3590ebc040c9e ("NFC: logging neatening")
    3682f49f32051 ("NFC: netlink: Add new netlink command NFC_CMD_ACTIVATE_TARGET")
    37f895d7e85e7 ("NFC: pn533: Fix socket deadlock")
    38986ffa6a748 ("xhci: use port structures instead of port arrays in xhci.c functions")
    53460c53b7619 ("[media] au0828: Add timer to restart TS stream if no data arrives on bulk endpoint")
    5df848f37b1d2 ("NFC: pn533: fix error return code")
    7c96f59e0cafe ("[media] s5p-mfc: Fix initialization of internal structures")
    80c1bce9aa315 ("[media] au0828: Refactoring for start_urb_transfer()")
    9295b5b569fc4 ("NFC: nci: Add support for different NCI_DEACTIVATE_TYPE")
    96d4581f0b371 ("NFC: netlink: Add mode parameter to deactivate_target functions")
    9815c7cf22dac ("NFC: pn533: Separate physical layer from the core implementation")
    b9eaf18722221 ("treewide: init_timer() -> setup_timer()")
    d7979e130ebb0 ("NFC: NCI: Signal deactivation in Target mode")
    e997ebbe46fe4 ("NFC: pn533: Send ATR_REQ only if NFC_PROTO_NFC_DEP bit is set")
    e99e88a9d2b06 ("treewide: setup_timer() -> timer_setup()")


How should we proceed with this patch?

--
Thanks,
Sasha

[-- Attachment #2: 0001-usb-xhci-avoid-null-pointer-deref-when-bos-field-is-.patch.4.14 --]
[-- Type: application/octet-stream, Size: 4900 bytes --]

From b1ceb3787bcd6f3ae775eebead9a66b80a2b4314 Mon Sep 17 00:00:00 2001
From: Carsten Schmid <carsten_schmid@mentor.com>
Date: Fri, 8 Mar 2019 15:15:52 +0100
Subject: [PATCH] usb: xhci: avoid null pointer deref when bos field is NULL

With defective USB sticks we see the following error happen:
usb 1-3: new high-speed USB device number 6 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: device descriptor read/64, error -71
usb 1-3: new high-speed USB device number 7 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: unable to get BOS descriptor set
usb 1-3: New USB device found, idVendor=0781, idProduct=5581
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
...
BUG: unable to handle kernel NULL pointer dereference at 0000000000000008

This comes from the following place:
[ 1660.215380] IP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.222092] PGD 0 P4D 0
[ 1660.224918] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 1660.425520] CPU: 1 PID: 38 Comm: kworker/1:1 Tainted: P     U  W  O    4.14.67-apl #1
[ 1660.434277] Workqueue: usb_hub_wq hub_event [usbcore]
[ 1660.439918] task: ffffa295b6ae4c80 task.stack: ffffad4580150000
[ 1660.446532] RIP: 0010:xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.453821] RSP: 0018:ffffad4580153c70 EFLAGS: 00010046
[ 1660.459655] RAX: 0000000000000000 RBX: ffffa295b4d7c000 RCX: 0000000000000002
[ 1660.467625] RDX: 0000000000000002 RSI: ffffffff984a55b2 RDI: ffffffff984a55b2
[ 1660.475586] RBP: ffffad4580153cc8 R08: 0000000000d6520a R09: 0000000000000001
[ 1660.483556] R10: ffffad4580a004a0 R11: 0000000000000286 R12: ffffa295b4d7c000
[ 1660.491525] R13: 0000000000010648 R14: ffffa295a84e1800 R15: 0000000000000000
[ 1660.499494] FS:  0000000000000000(0000) GS:ffffa295bfc80000(0000) knlGS:0000000000000000
[ 1660.508530] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1660.514947] CR2: 0000000000000008 CR3: 000000025a114000 CR4: 00000000003406a0
[ 1660.522917] Call Trace:
[ 1660.525657]  usb_set_usb2_hardware_lpm+0x3d/0x70 [usbcore]
[ 1660.531792]  usb_disable_device+0x242/0x260 [usbcore]
[ 1660.537439]  usb_disconnect+0xc1/0x2b0 [usbcore]
[ 1660.542600]  hub_event+0x596/0x18f0 [usbcore]
[ 1660.547467]  ? trace_preempt_on+0xdf/0x100
[ 1660.552040]  ? process_one_work+0x1c1/0x410
[ 1660.556708]  process_one_work+0x1d2/0x410
[ 1660.561184]  ? preempt_count_add.part.3+0x21/0x60
[ 1660.566436]  worker_thread+0x2d/0x3f0
[ 1660.570522]  kthread+0x122/0x140
[ 1660.574123]  ? process_one_work+0x410/0x410
[ 1660.578792]  ? kthread_create_on_node+0x60/0x60
[ 1660.583849]  ret_from_fork+0x3a/0x50
[ 1660.587839] Code: 00 49 89 c3 49 8b 84 24 50 16 00 00 8d 4a ff 48 8d 04 c8 48 89 ca 4c 8b 10 45 8b 6a 04 48 8b 00 48 89 45 c0 49 8b 86 80 03 00 00 <48> 8b 40 08 8b 40 03 0f 1f 44 00 00 45 85 ff 0f 84 81 01 00 00
[ 1660.608980] RIP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd] RSP: ffffad4580153c70
[ 1660.617921] CR2: 0000000000000008

Tracking this down shows that udev->bos is NULL in the following code:
(xhci.c, in xhci_set_usb2_hardware_lpm)
	field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);  <<<<<<< here

	xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
			enable ? "enable" : "disable", port_num + 1);

	if (enable) {
		/* Host supports BESL timeout instead of HIRD */
		if (udev->usb2_hw_lpm_besl_capable) {
			/* if device doesn't have a preferred BESL value use a
			 * default one which works with mixed HIRD and BESL
			 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
			 */
			if ((field & USB_BESL_SUPPORT) &&
			    (field & USB_BESL_BASELINE_VALID))
				hird = USB_GET_BESL_BASELINE(field);
			else
				hird = udev->l1_params.besl;

The failing case is when disabling LPM. So it is sufficient to avoid
access to udev->bos by moving the instruction into the "enable" clause.

Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c78de07c4d00..6001c3cefab3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4155,7 +4155,6 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 	pm_addr = port_array[port_num] + PORTPMSC;
 	pm_val = readl(pm_addr);
 	hlpm_addr = port_array[port_num] + PORTHLPMC;
-	field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 
 	xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
 			enable ? "enable" : "disable", port_num + 1);
@@ -4167,6 +4166,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 			 * default one which works with mixed HIRD and BESL
 			 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
 			 */
+			field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 			if ((field & USB_BESL_SUPPORT) &&
 			    (field & USB_BESL_BASELINE_VALID))
 				hird = USB_GET_BESL_BASELINE(field);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* AW: [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL
  2019-06-03 15:00     ` Schmid, Carsten
@ 2019-06-04  6:52       ` Schmid, Carsten
  0 siblings, 0 replies; 13+ messages in thread
From: Schmid, Carsten @ 2019-06-04  6:52 UTC (permalink / raw)
  To: Sasha Levin, Mathias Nyman, gregkh; +Cc: linux-usb, Stable, Stable

> Hi Sasha,
> 
> i have (back)ported the patch to the older kernels mentioned below
> where the original patch failed.
> 
> The patch appended to this mail applies to v4.14.121, v4.9.178, v4.4.180 and
> v3.18.140.
> Some changes within the xhci driver prevented git from finding the correct
> position.
> 
> Hope this helps :-)
> 
> Best regards
> Carsten
> 
Brrrr. IT stuff changed so i append the patch as text:
---
From b1ceb3787bcd6f3ae775eebead9a66b80a2b4314 Mon Sep 17 00:00:00 2001
From: Carsten Schmid <carsten_schmid@mentor.com>
Date: Fri, 8 Mar 2019 15:15:52 +0100
Subject: [PATCH] usb: xhci: avoid null pointer deref when bos field is NULL

With defective USB sticks we see the following error happen:
usb 1-3: new high-speed USB device number 6 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: device descriptor read/64, error -71
usb 1-3: new high-speed USB device number 7 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: unable to get BOS descriptor set
usb 1-3: New USB device found, idVendor=0781, idProduct=5581
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
...
BUG: unable to handle kernel NULL pointer dereference at 0000000000000008

This comes from the following place:
[ 1660.215380] IP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.222092] PGD 0 P4D 0
[ 1660.224918] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 1660.425520] CPU: 1 PID: 38 Comm: kworker/1:1 Tainted: P     U  W  O    4.14.67-apl #1
[ 1660.434277] Workqueue: usb_hub_wq hub_event [usbcore]
[ 1660.439918] task: ffffa295b6ae4c80 task.stack: ffffad4580150000
[ 1660.446532] RIP: 0010:xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.453821] RSP: 0018:ffffad4580153c70 EFLAGS: 00010046
[ 1660.459655] RAX: 0000000000000000 RBX: ffffa295b4d7c000 RCX: 0000000000000002
[ 1660.467625] RDX: 0000000000000002 RSI: ffffffff984a55b2 RDI: ffffffff984a55b2
[ 1660.475586] RBP: ffffad4580153cc8 R08: 0000000000d6520a R09: 0000000000000001
[ 1660.483556] R10: ffffad4580a004a0 R11: 0000000000000286 R12: ffffa295b4d7c000
[ 1660.491525] R13: 0000000000010648 R14: ffffa295a84e1800 R15: 0000000000000000
[ 1660.499494] FS:  0000000000000000(0000) GS:ffffa295bfc80000(0000) knlGS:0000000000000000
[ 1660.508530] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1660.514947] CR2: 0000000000000008 CR3: 000000025a114000 CR4: 00000000003406a0
[ 1660.522917] Call Trace:
[ 1660.525657]  usb_set_usb2_hardware_lpm+0x3d/0x70 [usbcore]
[ 1660.531792]  usb_disable_device+0x242/0x260 [usbcore]
[ 1660.537439]  usb_disconnect+0xc1/0x2b0 [usbcore]
[ 1660.542600]  hub_event+0x596/0x18f0 [usbcore]
[ 1660.547467]  ? trace_preempt_on+0xdf/0x100
[ 1660.552040]  ? process_one_work+0x1c1/0x410
[ 1660.556708]  process_one_work+0x1d2/0x410
[ 1660.561184]  ? preempt_count_add.part.3+0x21/0x60
[ 1660.566436]  worker_thread+0x2d/0x3f0
[ 1660.570522]  kthread+0x122/0x140
[ 1660.574123]  ? process_one_work+0x410/0x410
[ 1660.578792]  ? kthread_create_on_node+0x60/0x60
[ 1660.583849]  ret_from_fork+0x3a/0x50
[ 1660.587839] Code: 00 49 89 c3 49 8b 84 24 50 16 00 00 8d 4a ff 48 8d 04 c8 48 89 ca 4c 8b 10 45 8b 6a 04 48 8b 00 48 89 45 c0 49 8b 86 80 03 00 00 <48> 8b 40 08 8b 40 03 0f 1f 44 00 00 45 85 ff 0f 84 81 01 00 00
[ 1660.608980] RIP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd] RSP: ffffad4580153c70
[ 1660.617921] CR2: 0000000000000008

Tracking this down shows that udev->bos is NULL in the following code:
(xhci.c, in xhci_set_usb2_hardware_lpm)
	field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);  <<<<<<< here

	xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
			enable ? "enable" : "disable", port_num + 1);

	if (enable) {
		/* Host supports BESL timeout instead of HIRD */
		if (udev->usb2_hw_lpm_besl_capable) {
			/* if device doesn't have a preferred BESL value use a
			 * default one which works with mixed HIRD and BESL
			 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
			 */
			if ((field & USB_BESL_SUPPORT) &&
			    (field & USB_BESL_BASELINE_VALID))
				hird = USB_GET_BESL_BASELINE(field);
			else
				hird = udev->l1_params.besl;

The failing case is when disabling LPM. So it is sufficient to avoid
access to udev->bos by moving the instruction into the "enable" clause.

Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c78de07c4d00..6001c3cefab3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4155,7 +4155,6 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 	pm_addr = port_array[port_num] + PORTPMSC;
 	pm_val = readl(pm_addr);
 	hlpm_addr = port_array[port_num] + PORTHLPMC;
-	field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 
 	xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
 			enable ? "enable" : "disable", port_num + 1);
@@ -4167,6 +4166,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 			 * default one which works with mixed HIRD and BESL
 			 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
 			 */
+			field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 			if ((field & USB_BESL_SUPPORT) &&
 			    (field & USB_BESL_BASELINE_VALID))
 				hird = USB_GET_BESL_BASELINE(field);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/5] xhci fixes for usb-linus
  2021-10-08  9:25 Mathias Nyman
@ 2021-10-08 16:13 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2021-10-08 16:13 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: linux-usb

On Fri, Oct 08, 2021 at 12:25:42PM +0300, Mathias Nyman wrote:
> Hi Greg
> 
> A few xhci fixes for 5.15-rc.
> A couple races fixed, and quirks added for a couple hosts.

I'll pick these up after Linus takes my latest pull request, these were
too late to get into linux-next this week for testing.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/5] xhci fixes for usb-linus
@ 2021-10-08  9:25 Mathias Nyman
  2021-10-08 16:13 ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Mathias Nyman @ 2021-10-08  9:25 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci fixes for 5.15-rc.
A couple races fixed, and quirks added for a couple hosts.

Thanks
-Mathias


Johan Hovold (1):
  USB: xhci: dbc: fix tty registration race

Jonathan Bell (2):
  xhci: guard accesses to ep_state in xhci_endpoint_reset()
  xhci: add quirk for host controllers that don't update endpoint DCS

Nikolay Martynov (1):
  xhci: Enable trust tx length quirk for Fresco FL11 USB controller

Pavankumar Kondeti (1):
  xhci: Fix command ring pointer corruption while aborting a command

 drivers/usb/host/xhci-dbgtty.c | 28 ++++++++++++------------
 drivers/usb/host/xhci-pci.c    |  6 +++++-
 drivers/usb/host/xhci-ring.c   | 39 +++++++++++++++++++++++++++++-----
 drivers/usb/host/xhci.c        |  5 +++++
 drivers/usb/host/xhci.h        |  1 +
 5 files changed, 58 insertions(+), 21 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/5] xhci fixes for usb-linus
@ 2021-05-12  8:08 Mathias Nyman
  0 siblings, 0 replies; 13+ messages in thread
From: Mathias Nyman @ 2021-05-12  8:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few small xHCI fixes for usb-linus 5.13-rc.

Thanks
-Mathias


Abhijeet Rao (1):
  xhci-pci: Allow host runtime PM as default for Intel Alder Lake xHCI

Christophe JAILLET (1):
  xhci: Do not use GFP_KERNEL in (potentially) atomic context

Mathias Nyman (1):
  xhci: Fix giving back cancelled URBs even if halted endpoint can't
    reset

Maximilian Luz (1):
  usb: xhci: Increase timeout for HC halt

Sandeep Singh (1):
  xhci: Add reset resume quirk for AMD xhci controller.

 drivers/usb/host/xhci-ext-caps.h |  5 +++--
 drivers/usb/host/xhci-pci.c      |  8 ++++++--
 drivers/usb/host/xhci-ring.c     | 16 +++++++++++-----
 drivers/usb/host/xhci.c          |  6 +++---
 4 files changed, 23 insertions(+), 12 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/5] xhci fixes for usb-linus
@ 2020-06-24 13:59 Mathias Nyman
  0 siblings, 0 replies; 13+ messages in thread
From: Mathias Nyman @ 2020-06-24 13:59 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci fixes for usb-linus that resolve a couple power management
related issues, and a full-speed USB device enumeration problem.

-Mathias

Al Cooper (1):
  xhci: Fix enumeration issue when setting max packet size for FS
    devices.

Kai-Heng Feng (2):
  xhci: Return if xHCI doesn't support LPM
  xhci: Poll for U0 after disabling USB2 LPM

Macpaul Lin (1):
  usb: host: xhci-mtk: avoid runtime suspend when removing hcd

Mathias Nyman (1):
  xhci: Fix incorrect EP_STATE_MASK

 drivers/usb/host/xhci-mtk.c | 5 +++--
 drivers/usb/host/xhci.c     | 9 ++++++++-
 drivers/usb/host/xhci.h     | 2 +-
 3 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-10-08 16:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22 11:33 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
2019-05-22 11:33 ` [PATCH 1/5] xhci: update bounce buffer with correct sg num Mathias Nyman
2019-05-22 11:33 ` [PATCH 2/5] usb: xhci: Fix a potential null pointer dereference in xhci_debugfs_create_endpoint() Mathias Nyman
2019-05-22 11:33 ` [PATCH 3/5] usb: xhci: avoid null pointer deref when bos field is NULL Mathias Nyman
     [not found]   ` <20190529131455.C7A8C217D4@mail.kernel.org>
2019-06-03  7:12     ` AW: " Schmid, Carsten
2019-06-03 15:00     ` Schmid, Carsten
2019-06-04  6:52       ` Schmid, Carsten
2019-05-22 11:34 ` [PATCH 4/5] xhci: Fix immediate data transfer if buffer is already DMA mapped Mathias Nyman
2019-05-22 11:34 ` [PATCH 5/5] xhci: Convert xhci_handshake() to use readl_poll_timeout_atomic() Mathias Nyman
2020-06-24 13:59 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
2021-05-12  8:08 Mathias Nyman
2021-10-08  9:25 Mathias Nyman
2021-10-08 16:13 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).