All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] xhci features for usb-next
@ 2021-04-06  7:02 Mathias Nyman
  2021-04-06  7:02 ` [PATCH 1/4] xhci: check port array allocation was successful before dereferencing it Mathias Nyman
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Mathias Nyman @ 2021-04-06  7:02 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

I saw you already picked the Mediatek xhci patches.

Here are a few additional patches I had pending for usb-next
but didn't get around to submit before easter.

Mostly fixing potential issues found by fuzzer and other tools.

Thanks
-Mathias

Mathias Nyman (4):
  xhci: check port array allocation was successful before dereferencing
    it
  xhci: check control context is valid before dereferencing it.
  xhci: fix potential array out of bounds with several interrupters
  xhci: prevent double-fetch of transfer and transfer event TRBs

 drivers/usb/host/xhci-mem.c  |  3 +++
 drivers/usb/host/xhci-ring.c | 42 ++++++++++++++++--------------------
 drivers/usb/host/xhci.c      | 14 +++++++++++-
 3 files changed, 35 insertions(+), 24 deletions(-)

-- 
2.25.1


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

* [PATCH 1/4] xhci: check port array allocation was successful before dereferencing it
  2021-04-06  7:02 [PATCH 0/4] xhci features for usb-next Mathias Nyman
@ 2021-04-06  7:02 ` Mathias Nyman
  2021-04-06  7:02 ` [PATCH 2/4] xhci: check control context is valid " Mathias Nyman
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2021-04-06  7:02 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

return if rhub->ports is null after rhub->ports = kcalloc_node()
Klockwork reported issue

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 34d95c006751..f66815fe8482 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2249,6 +2249,9 @@ static void xhci_create_rhub_port_array(struct xhci_hcd *xhci,
 		return;
 	rhub->ports = kcalloc_node(rhub->num_ports, sizeof(*rhub->ports),
 			flags, dev_to_node(dev));
+	if (!rhub->ports)
+		return;
+
 	for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
 		if (xhci->hw_ports[i].rhub != rhub ||
 		    xhci->hw_ports[i].hcd_portnum == DUPLICATE_ENTRY)
-- 
2.25.1


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

* [PATCH 2/4] xhci: check control context is valid before dereferencing it.
  2021-04-06  7:02 [PATCH 0/4] xhci features for usb-next Mathias Nyman
  2021-04-06  7:02 ` [PATCH 1/4] xhci: check port array allocation was successful before dereferencing it Mathias Nyman
@ 2021-04-06  7:02 ` Mathias Nyman
  2021-04-06  7:02 ` [PATCH 3/4] xhci: fix potential array out of bounds with several interrupters Mathias Nyman
  2021-04-06  7:02 ` [PATCH 4/4] xhci: prevent double-fetch of transfer and transfer event TRBs Mathias Nyman
  3 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2021-04-06  7:02 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Don't dereference ctrl_ctx before checking it's valid.
Issue reported by Klockwork

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5d9fc3cd07a5..f9614716ecd7 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3261,6 +3261,14 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 
 	/* config ep command clears toggle if add and drop ep flags are set */
 	ctrl_ctx = xhci_get_input_control_ctx(cfg_cmd->in_ctx);
+	if (!ctrl_ctx) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		xhci_free_command(xhci, cfg_cmd);
+		xhci_warn(xhci, "%s: Could not get input context, bad type.\n",
+				__func__);
+		goto cleanup;
+	}
+
 	xhci_setup_input_ctx_for_config_ep(xhci, cfg_cmd->in_ctx, vdev->out_ctx,
 					   ctrl_ctx, ep_flag, ep_flag);
 	xhci_endpoint_copy(xhci, cfg_cmd->in_ctx, vdev->out_ctx, ep_index);
-- 
2.25.1


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

* [PATCH 3/4] xhci: fix potential array out of bounds with several interrupters
  2021-04-06  7:02 [PATCH 0/4] xhci features for usb-next Mathias Nyman
  2021-04-06  7:02 ` [PATCH 1/4] xhci: check port array allocation was successful before dereferencing it Mathias Nyman
  2021-04-06  7:02 ` [PATCH 2/4] xhci: check control context is valid " Mathias Nyman
@ 2021-04-06  7:02 ` Mathias Nyman
  2021-04-06  7:02 ` [PATCH 4/4] xhci: prevent double-fetch of transfer and transfer event TRBs Mathias Nyman
  3 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2021-04-06  7:02 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

The Max Interrupters supported by the controller is given in a 10bit
wide bitfield, but the driver uses a fixed 128 size array to index these
interrupters.

Klockwork reports a possible array out of bounds case which in theory
is possible. In practice this hasn't been hit as a common number of Max
Interrupters for new controllers is 8, not even close to 128.

This needs to be fixed anyway

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f9614716ecd7..ca9385d22f68 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -227,6 +227,7 @@ static void xhci_zero_64b_regs(struct xhci_hcd *xhci)
 	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
 	int err, i;
 	u64 val;
+	u32 intrs;
 
 	/*
 	 * Some Renesas controllers get into a weird state if they are
@@ -265,7 +266,10 @@ static void xhci_zero_64b_regs(struct xhci_hcd *xhci)
 	if (upper_32_bits(val))
 		xhci_write_64(xhci, 0, &xhci->op_regs->cmd_ring);
 
-	for (i = 0; i < HCS_MAX_INTRS(xhci->hcs_params1); i++) {
+	intrs = min_t(u32, HCS_MAX_INTRS(xhci->hcs_params1),
+		      ARRAY_SIZE(xhci->run_regs->ir_set));
+
+	for (i = 0; i < intrs; i++) {
 		struct xhci_intr_reg __iomem *ir;
 
 		ir = &xhci->run_regs->ir_set[i];
-- 
2.25.1


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

* [PATCH 4/4] xhci: prevent double-fetch of transfer and transfer event TRBs
  2021-04-06  7:02 [PATCH 0/4] xhci features for usb-next Mathias Nyman
                   ` (2 preceding siblings ...)
  2021-04-06  7:02 ` [PATCH 3/4] xhci: fix potential array out of bounds with several interrupters Mathias Nyman
@ 2021-04-06  7:02 ` Mathias Nyman
  3 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2021-04-06  7:02 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

The same values are parsed several times from transfer and event
TRBs by different functions in the same call path, all while processing
one transfer event.

As the TRBs are in DMA memory and can be accessed by the xHC host we want
to avoid this to prevent double-fetch issues.

To resolve this pass the already parsed values to the different functions
in the path of parsing a transfer event

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 42 ++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1eee60ac518f..05c38dd3ee36 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2128,16 +2128,13 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code)
 	return 0;
 }
 
-static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
-	struct xhci_transfer_event *event, struct xhci_virt_ep *ep)
+static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+		     struct xhci_ring *ep_ring, struct xhci_td *td,
+		     u32 trb_comp_code)
 {
 	struct xhci_ep_ctx *ep_ctx;
-	struct xhci_ring *ep_ring;
-	u32 trb_comp_code;
 
-	ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));
 	ep_ctx = xhci_get_ep_ctx(xhci, ep->vdev->out_ctx, ep->ep_index);
-	trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
 
 	switch (trb_comp_code) {
 	case COMP_STOPPED_LENGTH_INVALID:
@@ -2233,9 +2230,9 @@ static int sum_trb_lengths(struct xhci_hcd *xhci, struct xhci_ring *ring,
 /*
  * Process control tds, update urb status and actual_length.
  */
-static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
-	union xhci_trb *ep_trb, struct xhci_transfer_event *event,
-	struct xhci_virt_ep *ep)
+static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+		struct xhci_ring *ep_ring,  struct xhci_td *td,
+			   union xhci_trb *ep_trb, struct xhci_transfer_event *event)
 {
 	struct xhci_ep_ctx *ep_ctx;
 	u32 trb_comp_code;
@@ -2323,15 +2320,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
 		td->urb->actual_length = requested;
 
 finish_td:
-	return finish_td(xhci, td, event, ep);
+	return finish_td(xhci, ep, ep_ring, td, trb_comp_code);
 }
 
 /*
  * Process isochronous tds, update urb packet status and actual_length.
  */
-static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
-	union xhci_trb *ep_trb, struct xhci_transfer_event *event,
-	struct xhci_virt_ep *ep)
+static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+		struct xhci_ring *ep_ring, struct xhci_td *td,
+		union xhci_trb *ep_trb, struct xhci_transfer_event *event)
 {
 	struct urb_priv *urb_priv;
 	int idx;
@@ -2408,7 +2405,7 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
 
 	td->urb->actual_length += frame->actual_length;
 
-	return finish_td(xhci, td, event, ep);
+	return finish_td(xhci, ep, ep_ring, td, trb_comp_code);
 }
 
 static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
@@ -2440,17 +2437,15 @@ static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
 /*
  * Process bulk and interrupt tds, update urb status and actual_length.
  */
-static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td,
-	union xhci_trb *ep_trb, struct xhci_transfer_event *event,
-	struct xhci_virt_ep *ep)
+static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+		struct xhci_ring *ep_ring, struct xhci_td *td,
+		union xhci_trb *ep_trb, struct xhci_transfer_event *event)
 {
 	struct xhci_slot_ctx *slot_ctx;
-	struct xhci_ring *ep_ring;
 	u32 trb_comp_code;
 	u32 remaining, requested, ep_trb_len;
 
 	slot_ctx = xhci_get_slot_ctx(xhci, ep->vdev->out_ctx);
-	ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));
 	trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
 	remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
 	ep_trb_len = TRB_LEN(le32_to_cpu(ep_trb->generic.field[2]));
@@ -2510,7 +2505,8 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td,
 			  remaining);
 		td->urb->actual_length = 0;
 	}
-	return finish_td(xhci, td, event, ep);
+
+	return finish_td(xhci, ep, ep_ring, td, trb_comp_code);
 }
 
 /*
@@ -2853,11 +2849,11 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 
 		/* update the urb's actual_length and give back to the core */
 		if (usb_endpoint_xfer_control(&td->urb->ep->desc))
-			process_ctrl_td(xhci, td, ep_trb, event, ep);
+			process_ctrl_td(xhci, ep, ep_ring, td, ep_trb, event);
 		else if (usb_endpoint_xfer_isoc(&td->urb->ep->desc))
-			process_isoc_td(xhci, td, ep_trb, event, ep);
+			process_isoc_td(xhci, ep, ep_ring, td, ep_trb, event);
 		else
-			process_bulk_intr_td(xhci, td, ep_trb, event, ep);
+			process_bulk_intr_td(xhci, ep, ep_ring, td, ep_trb, event);
 cleanup:
 		handling_skipped_tds = ep->skip &&
 			trb_comp_code != COMP_MISSED_SERVICE_ERROR &&
-- 
2.25.1


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

* Re: [PATCH 0/4] xhci features for usb-next
  2021-06-17 15:03 [PATCH 0/4] xhci features for usb-next Mathias Nyman
@ 2021-06-17 15:34 ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2021-06-17 15:34 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: linux-usb

On Thu, Jun 17, 2021 at 06:03:50PM +0300, Mathias Nyman wrote:
> Hi Greg
> 
> A few small patches for usb-next.
> 
> There's one double free fix here as well that I normally would send to
> usb-linus, but we're late in the cycle and this issue should be rare.
> It has been there since 5.6 and requires system to be out of memory, so
> I thought it can be added this way.

Yes, that's fine, all now applied, thanks.

greg k-h

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

* [PATCH 0/4] xhci features for usb-next
@ 2021-06-17 15:03 Mathias Nyman
  2021-06-17 15:34 ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Mathias Nyman @ 2021-06-17 15:03 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few small patches for usb-next.

There's one double free fix here as well that I normally would send to
usb-linus, but we're late in the cycle and this issue should be rare.
It has been there since 5.6 and requires system to be out of memory, so
I thought it can be added this way.

Thanks
-Mathias

Mathias Nyman (3):
  xhci: Remove unused defines for ERST_SIZE and ERST_ENTRIES
  xhci: Add adaptive interrupt rate for isoch TRBs with XHCI_AVOID_BEI
    quirk
  xhci: handle failed buffer copy to URB sg list and fix a W=1 copiler
    warning

Zhangjiantao (Kirin, nanjing) (1):
  xhci: solve a double free problem while doing s4

 drivers/usb/host/xhci-mem.c  |  3 +++
 drivers/usb/host/xhci-ring.c |  7 ++++++-
 drivers/usb/host/xhci.c      |  9 +++++++--
 drivers/usb/host/xhci.h      | 11 +++++++----
 4 files changed, 23 insertions(+), 7 deletions(-)

-- 
2.25.1


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

* [PATCH 0/4] xhci features for usb-next
@ 2019-11-15 16:49 Mathias Nyman
  0 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2019-11-15 16:49 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci features for usb-next, hope they are not too late for 5.5 kernel.
These features mainly prepare driver for handling a flood of xhci events,
but also enables runtime PM as default for a Ice Lake xHCI, and adds a bit
of tracing.

-Mathias

Mathias Nyman (1):
  xhci: Add tracing for xhci doorbell register writes

Mika Westerberg (1):
  xhci-pci: Allow host runtime PM as default also for Intel Ice Lake
    xHCI

Peter Chen (1):
  usb: host: xhci: update event ring dequeue pointer on purpose

Suwan Kim (1):
  usb: host: xhci: Support running urb giveback in tasklet context

 drivers/usb/host/xhci-pci.c   |  4 ++-
 drivers/usb/host/xhci-ring.c  | 68 +++++++++++++++++++++++++++++++------------
 drivers/usb/host/xhci-trace.h | 26 +++++++++++++++++
 drivers/usb/host/xhci.c       |  3 +-
 drivers/usb/host/xhci.h       | 29 ++++++++++++++++++
 5 files changed, 109 insertions(+), 21 deletions(-)

-- 
2.7.4


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

* [PATCH 0/4] xhci features for usb-next
@ 2019-08-30 13:39 Mathias Nyman
  0 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2019-08-30 13:39 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

Minor xhci tuneups for usb-next.
The memory leak fix might look like it belongs to usb-linus and stable,
but is really about a leak possibility on a very unlikely error path.
Nice to have it fixed, but not sure it's stable material.

-Mathias

Christophe JAILLET (2):
  usb: xhci: dbc: Simplify error handling in 'xhci_dbc_alloc_requests()'
  usb: xhci: dbc: Use GFP_KERNEL instead of GFP_ATOMIC in
    'xhci_dbc_alloc_requests()'

Ikjoon Jang (1):
  xhci: fix possible memleak on setup address fails.

Mathias Nyman (1):
  xhci: add TSP bitflag to TRB tracing

 drivers/usb/host/xhci-dbgtty.c | 4 ++--
 drivers/usb/host/xhci.c        | 3 ++-
 drivers/usb/host/xhci.h        | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.7.4


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

* [PATCH 0/4] xhci features for usb-next
@ 2019-04-26 13:23 Mathias Nyman
  0 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2019-04-26 13:23 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few features for usb next, mostly tracing and debugging features, but
also support for Immediate Data Transfer for small (up to 8 bytes)
data transfers

-Mathias

Mathias Nyman (3):
  xhci: add port and bus number to port dynamic debugging
  xhci: Add tracing for input control context
  usb: xhci: add endpoint context tracing when an endpoint is added

Nicolas Saenz Julienne (1):
  usb: xhci: add Immediate Data Transfer support

 drivers/usb/host/xhci-hub.c   | 44 +++++++++++++++++++++++++----------------
 drivers/usb/host/xhci-ring.c  | 24 ++++++++++++++++++----
 drivers/usb/host/xhci-trace.h | 30 ++++++++++++++++++++++++++++
 drivers/usb/host/xhci.c       | 40 +++++++++++++++++++++++++++++++++----
 drivers/usb/host/xhci.h       | 46 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 159 insertions(+), 25 deletions(-)

-- 
2.7.4


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

end of thread, other threads:[~2021-06-17 15:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06  7:02 [PATCH 0/4] xhci features for usb-next Mathias Nyman
2021-04-06  7:02 ` [PATCH 1/4] xhci: check port array allocation was successful before dereferencing it Mathias Nyman
2021-04-06  7:02 ` [PATCH 2/4] xhci: check control context is valid " Mathias Nyman
2021-04-06  7:02 ` [PATCH 3/4] xhci: fix potential array out of bounds with several interrupters Mathias Nyman
2021-04-06  7:02 ` [PATCH 4/4] xhci: prevent double-fetch of transfer and transfer event TRBs Mathias Nyman
  -- strict thread matches above, loose matches on Subject: below --
2021-06-17 15:03 [PATCH 0/4] xhci features for usb-next Mathias Nyman
2021-06-17 15:34 ` Greg KH
2019-11-15 16:49 Mathias Nyman
2019-08-30 13:39 Mathias Nyman
2019-04-26 13:23 Mathias Nyman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.