Hector Martin (1): xhci: Handle TD clearing for multiple streams case
Mathias Nyman (6): xhci: use xhci_td_cleanup() helper when giving back cancelled URBs xhci: split handling halted endpoints into two steps xhci: fix giving back URB with incorrect status regression in 5.12 xhci: Fix 5.12 regression of missing xHC cache clearing command after a Stall xhci: introduce a new move_dequeue_past_td() function to replace old code. xhci: Fix failure to give back some cached cancelled URBs.
Michal Pecio (1): usb: xhci: Fix TD invalidation under pending Set TR Dequeue
drivers/usb/host/xhci-ring.c | 385 ++++++++++++++++++++++++++--------- drivers/usb/host/xhci.c | 94 +-------- drivers/usb/host/xhci.h | 9 + 3 files changed, 299 insertions(+), 189 deletions(-)
From: Mathias Nyman mathias.nyman@linux.intel.com
mainline inclusion from mainline-v5.12-rc1 commit e1a298390e987ddeb767cad18d913cb2782fda15 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
use the existing xhci_td_cleanup() to give back cancelled TDs when a ring is stopped.
A minor change to make sure we don't try to remove an already removed td from the list is needed as cancelled TDs are already removed from the td_list immediatelty when it's cancelled.
Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20210129130044.206855-18-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5abd9e69391a..aeb929253a19 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -795,8 +795,10 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, urb->actual_length = 0; status = 0; } - list_del_init(&td->td_list); - /* Was this TD slated to be cancelled but completed anyway? */ + /* TD might be removed from td_list if we are giving back a cancelled URB */ + if (!list_empty(&td->td_list)) + list_del_init(&td->td_list); + /* Giving back a cancelled URB, or if a slated TD completed anyway */ if (!list_empty(&td->cancelled_td_list)) list_del_init(&td->cancelled_td_list);
@@ -995,15 +997,11 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, struct xhci_td, cancelled_td_list); list_del_init(&cur_td->cancelled_td_list);
- /* Clean up the cancelled URB */ /* Doesn't matter what we pass for status, since the core will * just overwrite it (because the URB has been unlinked). */ ep_ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb); - xhci_unmap_td_bounce_buffer(xhci, ep_ring, cur_td); - inc_td_cnt(cur_td->urb); - if (last_td_in_urb(cur_td)) - xhci_giveback_urb_in_irq(xhci, cur_td, 0); + xhci_td_cleanup(xhci, cur_td, ep_ring, 0);
/* Stop processing the cancelled list if the watchdog timer is * running.
From: Mathias Nyman mathias.nyman@linux.intel.com
mainline inclusion from mainline-v5.12-rc1 commit 674f8438c12125d6b4fe51d44b9316bb02b286b5 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
Don't queue both a reset endpoint command and a set TR deq command at once when handling a halted endpoint.
split this into two steps. Initially only queue a reset endpoint command, and then if needed queue a set TR deq command in the reset endpoint handler.
Note: This removes the RESET_EP_QUIRK handling which was added in commit ac9d8fe7c6a8 ("USB: xhci: Add quirk for Fresco Logic xHCI hardware.")
This quirk was added in 2009 for prototype xHCI hardware meant for evaluation purposes only, and should not reach consumers. This hardware could not handle two commands queued at once, and had bad data in the output context after a reset endpoint command.
After this patch two command are no longer queued at once, so that part is solved in this rewrite, but the workaround for bad data in the output context solved by issuing an extra configure endpoint command is bluntly removed.
Adding this workaround to the new rewrite just adds complexity, and I think it's time to let this quirk go. Print a debug message instead.
Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20210129130044.206855-22-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Conflicts: drivers/usb/host/xhci-ring.c [Fix the conflicts caused by code context] Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 178 +++++++++++++++++------------------ drivers/usb/host/xhci.c | 94 ++---------------- drivers/usb/host/xhci.h | 8 ++ 3 files changed, 101 insertions(+), 179 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index aeb929253a19..7a57db4911f6 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -821,6 +821,30 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, return 0; }
+ +/* Complete the cancelled URBs we unlinked from td_list. */ +static void xhci_giveback_invalidated_tds(struct xhci_virt_ep *ep) +{ + struct xhci_ring *ring; + struct xhci_td *td, *tmp_td; + + list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, + cancelled_td_list) { + + /* + * Doesn't matter what we pass for status, since the core will + * just overwrite it (because the URB has been unlinked). + */ + ring = xhci_urb_to_transfer_ring(ep->xhci, td->urb); + + if (td->cancel_status == TD_CLEARED) + xhci_td_cleanup(ep->xhci, td, ring, 0); + + if (ep->xhci->xhc_state & XHCI_STATE_DYING) + return; + } +} + static int xhci_reset_halted_ep(struct xhci_hcd *xhci, unsigned int slot_id, unsigned int ep_index, enum xhci_ep_reset_type reset_type) { @@ -858,15 +882,19 @@ static void xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
ep->ep_state |= EP_HALTED;
+ /* add td to cancelled list and let reset ep handler take care of it */ + if (reset_type == EP_HARD_RESET) { + ep->ep_state |= EP_HARD_CLEAR_TOGGLE; + if (td && list_empty(&td->cancelled_td_list)) { + list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list); + td->cancel_status = TD_HALTED; + } + } + err = xhci_reset_halted_ep(xhci, slot_id, ep->ep_index, reset_type); if (err) return;
- if (reset_type == EP_HARD_RESET) { - ep->ep_state |= EP_HARD_CLEAR_TOGGLE; - xhci_cleanup_stalled_ring(xhci, slot_id, ep->ep_index, stream_id, - td); - } xhci_ring_cmd_db(xhci); }
@@ -875,16 +903,20 @@ static void xhci_handle_halted_endpoint(struct xhci_hcd *xhci, * We have the xHCI lock, so nothing can modify this list until we drop it. * We're also in the event handler, so we can't get re-interrupted if another * Stop Endpoint command completes. + * + * only call this when ring is not in a running state */
-static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep, - struct xhci_dequeue_state *deq_state) +static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) { struct xhci_hcd *xhci; struct xhci_td *td = NULL; struct xhci_td *tmp_td = NULL; + struct xhci_td *cached_td = NULL; struct xhci_ring *ring; + struct xhci_dequeue_state deq_state; u64 hw_deq; + unsigned int slot_id = ep->vdev->slot_id;
xhci = ep->xhci;
@@ -910,14 +942,28 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep,
if (trb_in_td(xhci, td->start_seg, td->first_trb, td->last_trb, hw_deq, false)) { - xhci_find_new_dequeue_state(xhci, ep->vdev->slot_id, - ep->ep_index, - td->urb->stream_id, - td, deq_state); + switch (td->cancel_status) { + case TD_CLEARED: /* TD is already no-op */ + case TD_CLEARING_CACHE: /* set TR deq command already queued */ + break; + case TD_DIRTY: /* TD is cached, clear it */ + case TD_HALTED: + /* FIXME stream case, several stopped rings */ + cached_td = td; + break; + } } else { td_to_noop(xhci, ring, td, false); + td->cancel_status = TD_CLEARED; } - + } + if (cached_td) { + cached_td->cancel_status = TD_CLEARING_CACHE; + xhci_find_new_dequeue_state(xhci, slot_id, ep->ep_index, + cached_td->urb->stream_id, + cached_td, &deq_state); + xhci_queue_new_dequeue_state(xhci, slot_id, ep->ep_index, + &deq_state); } return 0; } @@ -936,81 +982,32 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, union xhci_trb *trb, struct xhci_event_cmd *event) { unsigned int ep_index; - struct xhci_ring *ep_ring; struct xhci_virt_ep *ep; - struct xhci_td *cur_td = NULL; - struct xhci_td *last_unlinked_td; struct xhci_ep_ctx *ep_ctx; - struct xhci_virt_device *vdev; - struct xhci_dequeue_state deq_state;
if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb->generic.field[3])))) { if (!xhci->devs[slot_id]) - xhci_warn(xhci, "Stop endpoint command " - "completion for disabled slot %u\n", - slot_id); + xhci_warn(xhci, "Stop endpoint command completion for disabled slot %u\n", + slot_id); return; }
- memset(&deq_state, 0, sizeof(deq_state)); ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); - ep = xhci_get_virt_ep(xhci, slot_id, ep_index); if (!ep) return;
- vdev = xhci->devs[slot_id]; - ep_ctx = xhci_get_ep_ctx(xhci, vdev->out_ctx, ep_index); - trace_xhci_handle_cmd_stop_ep(ep_ctx); - - last_unlinked_td = list_last_entry(&ep->cancelled_td_list, - struct xhci_td, cancelled_td_list); + ep_ctx = xhci_get_ep_ctx(xhci, ep->vdev->out_ctx, ep_index);
- if (list_empty(&ep->cancelled_td_list)) { - xhci_stop_watchdog_timer_in_irq(xhci, ep); - ring_doorbell_for_active_rings(xhci, slot_id, ep_index); - return; - } - - xhci_invalidate_cancelled_tds(ep, &deq_state); + trace_xhci_handle_cmd_stop_ep(ep_ctx);
+ /* will queue a set TR deq if stopped on a cancelled, uncleared TD */ + xhci_invalidate_cancelled_tds(ep); xhci_stop_watchdog_timer_in_irq(xhci, ep);
- /* If necessary, queue a Set Transfer Ring Dequeue Pointer command */ - if (deq_state.new_deq_ptr && deq_state.new_deq_seg) { - xhci_queue_new_dequeue_state(xhci, slot_id, ep_index, - &deq_state); - xhci_ring_cmd_db(xhci); - } else { - /* Otherwise ring the doorbell(s) to restart queued transfers */ - ring_doorbell_for_active_rings(xhci, slot_id, ep_index); - } - - /* - * Drop the lock and complete the URBs in the cancelled TD list. - * New TDs to be cancelled might be added to the end of the list before - * we can complete all the URBs for the TDs we already unlinked. - * So stop when we've completed the URB for the last TD we unlinked. - */ - do { - cur_td = list_first_entry(&ep->cancelled_td_list, - struct xhci_td, cancelled_td_list); - list_del_init(&cur_td->cancelled_td_list); - - /* Doesn't matter what we pass for status, since the core will - * just overwrite it (because the URB has been unlinked). - */ - ep_ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb); - xhci_td_cleanup(xhci, cur_td, ep_ring, 0); - - /* Stop processing the cancelled list if the watchdog timer is - * running. - */ - if (xhci->xhc_state & XHCI_STATE_DYING) - return; - } while (cur_td != last_unlinked_td); - - /* Return to the event handler with xhci->lock re-acquired */ + /* Otherwise ring the doorbell(s) to restart queued transfers */ + xhci_giveback_invalidated_tds(ep); + ring_doorbell_for_active_rings(xhci, slot_id, ep_index); }
static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring) @@ -1231,6 +1228,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, struct xhci_virt_ep *ep; struct xhci_ep_ctx *ep_ctx; struct xhci_slot_ctx *slot_ctx; + struct xhci_td *td, *tmp_td;
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2])); @@ -1309,7 +1307,15 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, ep->queued_deq_seg, ep->queued_deq_ptr); } } - + /* HW cached TDs cleared from cache, give them back */ + list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, + cancelled_td_list) { + ep_ring = xhci_urb_to_transfer_ring(ep->xhci, td->urb); + if (td->cancel_status == TD_CLEARING_CACHE) { + td->cancel_status = TD_CLEARED; + xhci_td_cleanup(ep->xhci, td, ep_ring, td->status); + } + } cleanup: ep->ep_state &= ~SET_DEQ_PENDING; ep->queued_deq_seg = NULL; @@ -1341,27 +1347,15 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id, xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep, "Ignoring reset ep completion code of %u", cmd_comp_code);
- /* HW with the reset endpoint quirk needs to have a configure endpoint - * command complete before the endpoint can be used. Queue that here - * because the HW can't handle two commands being queued in a row. - */ - if (xhci->quirks & XHCI_RESET_EP_QUIRK) { - struct xhci_command *command; + /* Cleanup cancelled TDs as ep is stopped. May queue a Set TR Deq cmd */ + xhci_invalidate_cancelled_tds(ep);
- command = xhci_alloc_command(xhci, false, GFP_ATOMIC); - if (!command) - return; + if (xhci->quirks & XHCI_RESET_EP_QUIRK) + xhci_dbg(xhci, "Note: Removed workaround to queue config ep for this hw"); + /* Clear our internal halted state */ + ep->ep_state &= ~EP_HALTED;
- xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, - "Queueing configure endpoint command"); - xhci_queue_configure_endpoint(xhci, command, - xhci->devs[slot_id]->in_ctx->dma, slot_id, - false); - xhci_ring_cmd_db(xhci); - } else { - /* Clear our internal halted state */ - ep->ep_state &= ~EP_HALTED; - } + xhci_giveback_invalidated_tds(ep);
/* if this was a soft reset, then restart */ if ((le32_to_cpu(trb->generic.field[3])) & TRB_TSP) @@ -2093,7 +2087,9 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, xhci_clear_hub_tt_buffer(xhci, td, ep);
xhci_handle_halted_endpoint(xhci, ep, ep_ring->stream_id, td, - EP_HARD_RESET); + EP_HARD_RESET); + + return 0; /* xhci_handle_halted_endpoint marked td cancelled */ } else { /* Update ring dequeue pointer */ ep_ring->dequeue = td->last_trb; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index b4e5b15fbed9..ed52cc9e4df8 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1357,15 +1357,6 @@ static unsigned int xhci_get_endpoint_flag(struct usb_endpoint_descriptor *desc) return 1 << (xhci_get_endpoint_index(desc) + 1); }
-/* Find the flag for this endpoint (for use in the control context). Use the - * endpoint index to create a bitmask. The slot context is bit 0, endpoint 0 is - * bit 1, etc. - */ -static unsigned int xhci_get_endpoint_flag_from_index(unsigned int ep_index) -{ - return 1 << (ep_index + 1); -} - /* Compute the last valid endpoint context index. Basically, this is the * endpoint index plus one. For slot contexts with more than valid endpoint, * we find the most significant bit set in the added contexts flags. @@ -1732,7 +1723,12 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
for (; i < urb_priv->num_tds; i++) { td = &urb_priv->td[i]; - list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list); + /* TD can already be on cancelled list if ep halted on it */ + if (list_empty(&td->cancelled_td_list)) { + td->cancel_status = TD_DIRTY; + list_add_tail(&td->cancelled_td_list, + &ep->cancelled_td_list); + } }
/* Queue a stop endpoint command, but only if this is @@ -3041,84 +3037,6 @@ static void xhci_setup_input_ctx_for_config_ep(struct xhci_hcd *xhci, ctrl_ctx->add_flags |= cpu_to_le32(SLOT_FLAG); }
-static void xhci_setup_input_ctx_for_quirk(struct xhci_hcd *xhci, - unsigned int slot_id, unsigned int ep_index, - struct xhci_dequeue_state *deq_state) -{ - struct xhci_input_control_ctx *ctrl_ctx; - struct xhci_container_ctx *in_ctx; - struct xhci_ep_ctx *ep_ctx; - u32 added_ctxs; - dma_addr_t addr; - - in_ctx = xhci->devs[slot_id]->in_ctx; - ctrl_ctx = xhci_get_input_control_ctx(in_ctx); - if (!ctrl_ctx) { - xhci_warn(xhci, "%s: Could not get input context, bad type.\n", - __func__); - return; - } - - xhci_endpoint_copy(xhci, xhci->devs[slot_id]->in_ctx, - xhci->devs[slot_id]->out_ctx, ep_index); - ep_ctx = xhci_get_ep_ctx(xhci, in_ctx, ep_index); - addr = xhci_trb_virt_to_dma(deq_state->new_deq_seg, - deq_state->new_deq_ptr); - if (addr == 0) { - xhci_warn(xhci, "WARN Cannot submit config ep after " - "reset ep command\n"); - xhci_warn(xhci, "WARN deq seg = %p, deq ptr = %p\n", - deq_state->new_deq_seg, - deq_state->new_deq_ptr); - return; - } - ep_ctx->deq = cpu_to_le64(addr | deq_state->new_cycle_state); - - added_ctxs = xhci_get_endpoint_flag_from_index(ep_index); - xhci_setup_input_ctx_for_config_ep(xhci, xhci->devs[slot_id]->in_ctx, - xhci->devs[slot_id]->out_ctx, ctrl_ctx, - added_ctxs, added_ctxs); -} - -void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci, unsigned int slot_id, - unsigned int ep_index, unsigned int stream_id, - struct xhci_td *td) -{ - struct xhci_dequeue_state deq_state; - - xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep, - "Cleaning up stalled endpoint ring"); - /* We need to move the HW's dequeue pointer past this TD, - * or it will attempt to resend it on the next doorbell ring. - */ - xhci_find_new_dequeue_state(xhci, slot_id, ep_index, stream_id, td, - &deq_state); - - if (!deq_state.new_deq_ptr || !deq_state.new_deq_seg) - return; - - /* HW with the reset endpoint quirk will use the saved dequeue state to - * issue a configure endpoint command later. - */ - if (!(xhci->quirks & XHCI_RESET_EP_QUIRK)) { - xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep, - "Queueing new dequeue state"); - xhci_queue_new_dequeue_state(xhci, slot_id, - ep_index, &deq_state); - } else { - /* Better hope no one uses the input context between now and the - * reset endpoint completion! - * XXX: No idea how this hardware will react when stream rings - * are enabled. - */ - xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, - "Setting up input context for " - "configure endpoint command"); - xhci_setup_input_ctx_for_quirk(xhci, slot_id, - ep_index, &deq_state); - } -} - static void xhci_endpoint_disable(struct usb_hcd *hcd, struct usb_host_endpoint *host_ep) { diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index f8f93c6c4298..68f96e9e8a41 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1542,10 +1542,18 @@ struct xhci_segment { unsigned int bounce_len; };
+enum xhci_cancelled_td_status { + TD_DIRTY = 0, + TD_HALTED, + TD_CLEARING_CACHE, + TD_CLEARED, +}; + struct xhci_td { struct list_head td_list; struct list_head cancelled_td_list; int status; + enum xhci_cancelled_td_status cancel_status; struct urb *urb; struct xhci_segment *start_seg; union xhci_trb *first_trb;
From: Mathias Nyman mathias.nyman@linux.intel.com
mainline inclusion from mainline-v5.13-rc4 commit a80c203c3f1c06d2201c19ae071d0ae770a2b1ca category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
5.12 kernel changes how xhci handles cancelled URBs and halted endpoints. Among these changes cancelled and stalled URBs are no longer given back before they are cleared from xHC hardware cache.
These changes unfortunately cleared the -EPIPE status of a stalled transfer in one case before giving bak the URB, causing a USB card reader to fail from working.
Fixes: 674f8438c121 ("xhci: split handling halted endpoints into two steps") Cc: stable@vger.kernel.org # 5.12 Reported-by: Peter Ganzhorn peter.ganzhorn@googlemail.com Tested-by: Peter Ganzhorn peter.ganzhorn@googlemail.com Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20210525074100.1154090-2-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 7a57db4911f6..143f165a4908 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -831,14 +831,10 @@ static void xhci_giveback_invalidated_tds(struct xhci_virt_ep *ep) list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) {
- /* - * Doesn't matter what we pass for status, since the core will - * just overwrite it (because the URB has been unlinked). - */ ring = xhci_urb_to_transfer_ring(ep->xhci, td->urb);
if (td->cancel_status == TD_CLEARED) - xhci_td_cleanup(ep->xhci, td, ring, 0); + xhci_td_cleanup(ep->xhci, td, ring, td->status);
if (ep->xhci->xhc_state & XHCI_STATE_DYING) return;
From: Mathias Nyman mathias.nyman@linux.intel.com
mainline inclusion from mainline-v5.13-rc4 commit a7f2e9272aff1ccfe0fc801dab1d5a7a1c6b7ed2 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
If endpoints halts due to a stall then the dequeue pointer read from hardware may already be set ahead of the stalled TRB. After commit 674f8438c121 ("xhci: split handling halted endpoints into two steps") in 5.12 xhci driver won't issue a Set TR Dequeue if hardware dequeue pointer is already in the right place.
Turns out the "Set TR Dequeue pointer" command is anyway needed as it in addition to moving the dequeue pointer also clears endpoint state and cache.
Fixes: 674f8438c121 ("xhci: split handling halted endpoints into two steps") Cc: stable@vger.kernel.org # 5.12 Reported-by: Peter Ganzhorn peter.ganzhorn@googlemail.com Tested-by: Peter Ganzhorn peter.ganzhorn@googlemail.com Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20210525074100.1154090-3-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 143f165a4908..2ce554db1f09 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -929,14 +929,18 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) continue; } /* - * If ring stopped on the TD we need to cancel, then we have to + * If a ring stopped on the TD we need to cancel then we have to * move the xHC endpoint ring dequeue pointer past this TD. + * Rings halted due to STALL may show hw_deq is past the stalled + * TD, but still require a set TR Deq command to flush xHC cache. */ hw_deq = xhci_get_hw_deq(xhci, ep->vdev, ep->ep_index, td->urb->stream_id); hw_deq &= ~0xf;
- if (trb_in_td(xhci, td->start_seg, td->first_trb, + if (td->cancel_status == TD_HALTED) { + cached_td = td; + } else if (trb_in_td(xhci, td->start_seg, td->first_trb, td->last_trb, hw_deq, false)) { switch (td->cancel_status) { case TD_CLEARED: /* TD is already no-op */
From: Mathias Nyman mathias.nyman@linux.intel.com
mainline inclusion from mainline-v5.12-rc1 commit d1dbfb942c33bff563af7222418cff3f01c9fbc9 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
Replace xhci_find_new_dequeue_state() and xhci_queue_new_dequeue_state() functions with one combined function. These function were always called after each other, and had a lot of extra code just to pass the newly found dequeue state from the first function to the other.
The new function also returns error in case there is a failure to queue the new dequeue state. This way the caller can decide on recovery measures to handle it.
Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20210129130044.206855-25-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Conflicts: drivers/usb/host/xhci-ring.c [Fix the conflicts because of the code context] Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 151 +++++++++++++++++++++++++++++++++-- 1 file changed, 145 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 2ce554db1f09..0e916e2fc5ae 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -59,6 +59,10 @@ #include "xhci-trace.h" #include "xhci-mtk.h"
+static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, + u32 field1, u32 field2, + u32 field3, u32 field4, bool command_must_succeed); + /* * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA * address of the TRB. @@ -683,6 +687,136 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, (unsigned long long) addr); }
+static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci, + unsigned int slot_id, unsigned int ep_index, + unsigned int stream_id, struct xhci_td *td) +{ + struct xhci_virt_device *dev = xhci->devs[slot_id]; + struct xhci_virt_ep *ep = &dev->eps[ep_index]; + struct xhci_ring *ep_ring; + struct xhci_command *cmd; + struct xhci_segment *new_seg; + union xhci_trb *new_deq; + int new_cycle; + dma_addr_t addr; + u64 hw_dequeue; + bool cycle_found = false; + bool td_last_trb_found = false; + u32 trb_sct = 0; + int ret; + + ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id, + ep_index, stream_id); + if (!ep_ring) { + xhci_warn(xhci, "WARN can't find new dequeue, invalid stream ID %u\n", + stream_id); + return -ENODEV; + } + /* + * A cancelled TD can complete with a stall if HW cached the trb. + * In this case driver can't find td, but if the ring is empty we + * can move the dequeue pointer to the current enqueue position. + * We shouldn't hit this anymore as cached cancelled TRBs are given back + * after clearing the cache, but be on the safe side and keep it anyway + */ + if (!td) { + if (list_empty(&ep_ring->td_list)) { + new_seg = ep_ring->enq_seg; + new_deq = ep_ring->enqueue; + new_cycle = ep_ring->cycle_state; + xhci_dbg(xhci, "ep ring empty, Set new dequeue = enqueue"); + goto deq_found; + } else { + xhci_warn(xhci, "Can't find new dequeue state, missing td\n"); + return -EINVAL; + } + } + + hw_dequeue = xhci_get_hw_deq(xhci, dev, ep_index, stream_id); + new_seg = ep_ring->deq_seg; + new_deq = ep_ring->dequeue; + new_cycle = hw_dequeue & 0x1; + + /* + * We want to find the pointer, segment and cycle state of the new trb + * (the one after current TD's last_trb). We know the cycle state at + * hw_dequeue, so walk the ring until both hw_dequeue and last_trb are + * found. + */ + do { + if (!cycle_found && xhci_trb_virt_to_dma(new_seg, new_deq) + == (dma_addr_t)(hw_dequeue & ~0xf)) { + cycle_found = true; + if (td_last_trb_found) + break; + } + if (new_deq == td->last_trb) + td_last_trb_found = true; + + if (cycle_found && trb_is_link(new_deq) && + link_trb_toggles_cycle(new_deq)) + new_cycle ^= 0x1; + + next_trb(xhci, ep_ring, &new_seg, &new_deq); + + /* Search wrapped around, bail out */ + if (new_deq == ep->ring->dequeue) { + xhci_err(xhci, "Error: Failed finding new dequeue state\n"); + return -EINVAL; + } + + } while (!cycle_found || !td_last_trb_found); + +deq_found: + + /* Don't update the ring cycle state for the producer (us). */ + addr = xhci_trb_virt_to_dma(new_seg, new_deq); + if (addr == 0) { + xhci_warn(xhci, "Can't find dma of new dequeue ptr\n"); + xhci_warn(xhci, "deq seg = %p, deq ptr = %p\n", new_seg, new_deq); + return -EINVAL; + } + + if ((ep->ep_state & SET_DEQ_PENDING)) { + xhci_warn(xhci, "Set TR Deq already pending, don't submit for 0x%pad\n", + &addr); + return -EBUSY; + } + + /* This function gets called from contexts where it cannot sleep */ + cmd = xhci_alloc_command(xhci, false, GFP_ATOMIC); + if (!cmd) { + xhci_warn(xhci, "Can't alloc Set TR Deq cmd 0x%pad\n", &addr); + return -ENOMEM; + } + + if (stream_id) + trb_sct = SCT_FOR_TRB(SCT_PRI_TR); + ret = queue_command(xhci, cmd, + lower_32_bits(addr) | trb_sct | new_cycle, + upper_32_bits(addr), + STREAM_ID_FOR_TRB(stream_id), SLOT_ID_FOR_TRB(slot_id) | + EP_ID_FOR_TRB(ep_index) | TRB_TYPE(TRB_SET_DEQ), false); + if (ret < 0) { + xhci_free_command(xhci, cmd); + return ret; + } + ep->queued_deq_seg = new_seg; + ep->queued_deq_ptr = new_deq; + + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Set TR Deq ptr 0x%llx, cycle %u\n", addr, new_cycle); + + /* Stop the TD queueing code from ringing the doorbell until + * this command completes. The HC won't set the dequeue pointer + * if the ring is running, and ringing the doorbell starts the + * ring running. + */ + ep->ep_state |= SET_DEQ_PENDING; + xhci_ring_cmd_db(xhci); + return 0; +} + /* flip_cycle means flip the cycle bit of all but the first and last TRB. * (The last TRB actually points to the ring enqueue pointer, which is not part * of this TD.) This is used to remove partially enqueued isoc TDs from a ring. @@ -910,9 +1044,9 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) struct xhci_td *tmp_td = NULL; struct xhci_td *cached_td = NULL; struct xhci_ring *ring; - struct xhci_dequeue_state deq_state; u64 hw_deq; unsigned int slot_id = ep->vdev->slot_id; + int err;
xhci = ep->xhci;
@@ -959,11 +1093,16 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) } if (cached_td) { cached_td->cancel_status = TD_CLEARING_CACHE; - xhci_find_new_dequeue_state(xhci, slot_id, ep->ep_index, - cached_td->urb->stream_id, - cached_td, &deq_state); - xhci_queue_new_dequeue_state(xhci, slot_id, ep->ep_index, - &deq_state); + + err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index, + cached_td->urb->stream_id, + cached_td); + /* Failed to move past cached td, try just setting it noop */ + if (err) { + td_to_noop(xhci, ring, cached_td, false); + cached_td->cancel_status = TD_CLEARED; + } + cached_td = NULL; } return 0; }
From: Mathias Nyman mathias.nyman@linux.intel.com
mainline inclusion from mainline-v5.15-rc1 commit 94f339147fc3eb9edef7ee4ef6e39c569c073753 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
Only TDs with status TD_CLEARING_CACHE will be given back after cache is cleared with a set TR deq command.
xhci_invalidate_cached_td() failed to set the TD_CLEARING_CACHE status for some cancelled TDs as it assumed an endpoint only needs to clear the TD it stopped on.
This isn't always true. For example with streams enabled an endpoint may have several stream rings, each stopping on a different TDs.
Note that if an endpoint has several stream rings, the current code will still only clear the cache of the stream pointed to by the last cancelled TD in the cancel list.
This patch only focus on making sure all canceled TDs are given back, avoiding hung task after device removal. Another fix to solve clearing the caches of all stream rings with cancelled TDs is needed, but not as urgent.
This issue was simultanously discovered and debugged by by Tao Wang, with a slightly different fix proposal.
Fixes: 674f8438c121 ("xhci: split handling halted endpoints into two steps") Cc: stable@vger.kernel.org #5.12 Reported-by: Tao Wang wat@codeaurora.org Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20210820123503.2605901-4-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 40 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0e916e2fc5ae..8e1069f5b015 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1072,17 +1072,21 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) td->urb->stream_id); hw_deq &= ~0xf;
- if (td->cancel_status == TD_HALTED) { - cached_td = td; - } else if (trb_in_td(xhci, td->start_seg, td->first_trb, - td->last_trb, hw_deq, false)) { + if (td->cancel_status == TD_HALTED || + trb_in_td(xhci, td->start_seg, td->first_trb, td->last_trb, hw_deq, false)) { switch (td->cancel_status) { case TD_CLEARED: /* TD is already no-op */ case TD_CLEARING_CACHE: /* set TR deq command already queued */ break; case TD_DIRTY: /* TD is cached, clear it */ case TD_HALTED: - /* FIXME stream case, several stopped rings */ + td->cancel_status = TD_CLEARING_CACHE; + if (cached_td) + /* FIXME stream case, several stopped rings */ + xhci_dbg(xhci, + "Move dq past stream %u URB %p instead of stream %u URB %p\n", + td->urb->stream_id, td->urb, + cached_td->urb->stream_id, cached_td->urb); cached_td = td; break; } @@ -1091,18 +1095,24 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) td->cancel_status = TD_CLEARED; } } - if (cached_td) { - cached_td->cancel_status = TD_CLEARING_CACHE;
- err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index, - cached_td->urb->stream_id, - cached_td); - /* Failed to move past cached td, try just setting it noop */ - if (err) { - td_to_noop(xhci, ring, cached_td, false); - cached_td->cancel_status = TD_CLEARED; + /* If there's no need to move the dequeue pointer then we're done */ + if (!cached_td) + return 0; + + err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index, + cached_td->urb->stream_id, + cached_td); + if (err) { + /* Failed to move past cached td, just set cached TDs to no-op */ + list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) { + if (td->cancel_status != TD_CLEARING_CACHE) + continue; + xhci_dbg(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n", + td->urb); + td_to_noop(xhci, ring, td, false); + td->cancel_status = TD_CLEARED; } - cached_td = NULL; } return 0; }
From: Hector Martin marcan@marcan.st
mainline inclusion from mainline-v6.10-rc4 commit 5ceac4402f5d975e5a01c806438eb4e554771577 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
When multiple streams are in use, multiple TDs might be in flight when an endpoint is stopped. We need to issue a Set TR Dequeue Pointer for each, to ensure everything is reset properly and the caches cleared. Change the logic so that any N>1 TDs found active for different streams are deferred until after the first one is processed, calling xhci_invalidate_cancelled_tds() again from xhci_handle_cmd_set_deq() to queue another command until we are done with all of them. Also change the error/"should never happen" paths to ensure we at least clear any affected TDs, even if we can't issue a command to clear the hardware cache, and complain loudly with an xhci_warn() if this ever happens.
This problem case dates back to commit e9df17eb1408 ("USB: xhci: Correct assumptions about number of rings per endpoint.") early on in the XHCI driver's life, when stream support was first added. It was then identified but not fixed nor made into a warning in commit 674f8438c121 ("xhci: split handling halted endpoints into two steps"), which added a FIXME comment for the problem case (without materially changing the behavior as far as I can tell, though the new logic made the problem more obvious).
Then later, in commit 94f339147fc3 ("xhci: Fix failure to give back some cached cancelled URBs."), it was acknowledged again.
[Mathias: commit 94f339147fc3 ("xhci: Fix failure to give back some cached cancelled URBs.") was a targeted regression fix to the previously mentioned patch. Users reported issues with usb stuck after unmounting/disconnecting UAS devices. This rolled back the TD clearing of multiple streams to its original state.]
Apparently the commit author was aware of the problem (yet still chose to submit it): It was still mentioned as a FIXME, an xhci_dbg() was added to log the problem condition, and the remaining issue was mentioned in the commit description. The choice of making the log type xhci_dbg() for what is, at this point, a completely unhandled and known broken condition is puzzling and unfortunate, as it guarantees that no actual users would see the log in production, thereby making it nigh undebuggable (indeed, even if you turn on DEBUG, the message doesn't really hint at there being a problem at all).
It took me *months* of random xHC crashes to finally find a reliable repro and be able to do a deep dive debug session, which could all have been avoided had this unhandled, broken condition been actually reported with a warning, as it should have been as a bug intentionally left in unfixed (never mind that it shouldn't have been left in at all).
Another fix to solve clearing the caches of all stream rings with cancelled TDs is needed, but not as urgent.
3 years after that statement and 14 years after the original bug was introduced, I think it's finally time to fix it. And maybe next time let's not leave bugs unfixed (that are actually worse than the original bug), and let's actually get people to review kernel commits please.
Fixes xHC crashes and IOMMU faults with UAS devices when handling errors/faults. Easiest repro is to use `hdparm` to mark an early sector (e.g. 1024) on a disk as bad, then `cat /dev/sdX > /dev/null` in a loop. At least in the case of JMicron controllers, the read errors end up having to cancel two TDs (for two queued requests to different streams) and the one that didn't get cleared properly ends up faulting the xHC entirely when it tries to access DMA pages that have since been unmapped, referred to by the stale TDs. This normally happens quickly (after two or three loops). After this fix, I left the `cat` in a loop running overnight and experienced no xHC failures, with all read errors recovered properly. Repro'd and tested on an Apple M1 Mac Mini (dwc3 host).
On systems without an IOMMU, this bug would instead silently corrupt freed memory, making this a security bug (even on systems with IOMMUs this could silently corrupt memory belonging to other USB devices on the same controller, so it's still a security bug). Given that the kernel autoprobes partition tables, I'm pretty sure a malicious USB device pretending to be a UAS device and reporting an error with the right timing could deliberately trigger a UAF and write to freed memory, with no user action.
[Mathias: Commit message and code comment edit, original at:] https://lore.kernel.org/linux-usb/20240524-xhci-streams-v1-1-6b1f13819bea@ma...
Fixes: e9df17eb1408 ("USB: xhci: Correct assumptions about number of rings per endpoint.") Fixes: 94f339147fc3 ("xhci: Fix failure to give back some cached cancelled URBs.") Fixes: 674f8438c121 ("xhci: split handling halted endpoints into two steps") Cc: stable@vger.kernel.org Cc: security@kernel.org Reviewed-by: Neal Gompa neal@gompa.dev Signed-off-by: Hector Martin marcan@marcan.st Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20240611120610.3264502-5-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Conflicts: drivers/usb/host/xhci-ring.c [solve the confict because of the code context] Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 54 ++++++++++++++++++++++++++++-------- drivers/usb/host/xhci.h | 1 + 2 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 8e1069f5b015..4275c1a80819 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1080,13 +1080,27 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) break; case TD_DIRTY: /* TD is cached, clear it */ case TD_HALTED: + case TD_CLEARING_CACHE_DEFERRED: + if (cached_td) { + if (cached_td->urb->stream_id != td->urb->stream_id) { + /* Multiple streams case, defer move dq */ + xhci_dbg(xhci, + "Move dq deferred: stream %u URB %p\n", + td->urb->stream_id, td->urb); + td->cancel_status = TD_CLEARING_CACHE_DEFERRED; + break; + } + + /* Should never happen, but clear the TD if it does */ + xhci_warn(xhci, + "Found multiple active URBs %p and %p in stream %u?\n", + td->urb, cached_td->urb, + td->urb->stream_id); + td_to_noop(xhci, ring, cached_td, false); + cached_td->cancel_status = TD_CLEARED; + } + td->cancel_status = TD_CLEARING_CACHE; - if (cached_td) - /* FIXME stream case, several stopped rings */ - xhci_dbg(xhci, - "Move dq past stream %u URB %p instead of stream %u URB %p\n", - td->urb->stream_id, td->urb, - cached_td->urb->stream_id, cached_td->urb); cached_td = td; break; } @@ -1106,10 +1120,16 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) if (err) { /* Failed to move past cached td, just set cached TDs to no-op */ list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) { - if (td->cancel_status != TD_CLEARING_CACHE) + /* + * Deferred TDs need to have the deq pointer set after the above command + * completes, so if that failed we just give up on all of them (and + * complain loudly since this could cause issues due to caching). + */ + if (td->cancel_status != TD_CLEARING_CACHE && + td->cancel_status != TD_CLEARING_CACHE_DEFERRED) continue; - xhci_dbg(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n", - td->urb); + xhci_warn(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n", + td->urb); td_to_noop(xhci, ring, td, false); td->cancel_status = TD_CLEARED; } @@ -1378,6 +1398,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, struct xhci_ep_ctx *ep_ctx; struct xhci_slot_ctx *slot_ctx; struct xhci_td *td, *tmp_td; + bool deferred = false;
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2])); @@ -1463,14 +1484,25 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, if (td->cancel_status == TD_CLEARING_CACHE) { td->cancel_status = TD_CLEARED; xhci_td_cleanup(ep->xhci, td, ep_ring, td->status); + } else if (td->cancel_status == TD_CLEARING_CACHE_DEFERRED) { + deferred = true; } } cleanup: ep->ep_state &= ~SET_DEQ_PENDING; ep->queued_deq_seg = NULL; ep->queued_deq_ptr = NULL; - /* Restart any rings with pending URBs */ - ring_doorbell_for_active_rings(xhci, slot_id, ep_index); + + if (deferred) { + /* We have more streams to clear */ + xhci_dbg(ep->xhci, "%s: Pending TDs to clear, continuing with invalidation\n", + __func__); + xhci_invalidate_cancelled_tds(ep); + } else { + /* Restart any rings with pending URBs */ + xhci_dbg(ep->xhci, "%s: All TDs cleared, ring doorbell\n", __func__); + ring_doorbell_for_active_rings(xhci, slot_id, ep_index); + } }
static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id, diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 68f96e9e8a41..d82c90e73726 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1546,6 +1546,7 @@ enum xhci_cancelled_td_status { TD_DIRTY = 0, TD_HALTED, TD_CLEARING_CACHE, + TD_CLEARING_CACHE_DEFERRED, TD_CLEARED, };
From: Michal Pecio michal.pecio@gmail.com
mainline inclusion from mainline-v6.13-rc1 commit 484c3bab2d5dfa13ff659a51a06e9a393141eefc category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IACV8P CVE: CVE-2024-40927
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
xhci_invalidate_cancelled_tds() may not work correctly if the hardware is modifying endpoint or stream contexts at the same time by executing a Set TR Dequeue command. And even if it worked, it would be unable to queue Set TR Dequeue for the next stream, failing to clear xHC cache.
On stream endpoints, a chain of Set TR Dequeue commands may take some time to execute and we may want to cancel more TDs during this time. Currently this leads to Stop Endpoint completion handler calling this function without testing for SET_DEQ_PENDING, which will trigger the aforementioned problems when it happens.
On all endpoints, a halt condition causes Reset Endpoint to be queued and an error status given to the class driver, which may unlink more URBs in response. Stop Endpoint is queued and its handler may execute concurrently with Set TR Dequeue queued by Reset Endpoint handler.
(Reset Endpoint handler calls this function too, but there seems to be no possibility of it running concurrently with Set TR Dequeue).
Fix xhci_invalidate_cancelled_tds() to work correctly under a pending Set TR Dequeue. Bail out of the function when SET_DEQ_PENDING is set, then make the completion handler call the function again and also call xhci_giveback_invalidated_tds(), which needs to be called next.
This seems to fix another potential bug, where the handler would call xhci_invalidate_cancelled_tds(), which may clear some deferred TDs if a sanity check fails, and the TDs wouldn't be given back promptly.
Said sanity check seems to be wrong and prone to false positives when the endpoint halts, but fixing it is beyond the scope of this change, besides ensuring that cleared TDs are given back properly.
Fixes: 5ceac4402f5d ("xhci: Handle TD clearing for multiple streams case") CC: stable@vger.kernel.org Signed-off-by: Michal Pecio michal.pecio@gmail.com Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Link: https://lore.kernel.org/r/20241106101459.775897-33-mathias.nyman@linux.intel... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Conflicts: drivers/usb/host/xhci-ring.c [Only fix the code of context] Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- drivers/usb/host/xhci-ring.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 4275c1a80819..f5fbfc2a9b69 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1048,6 +1048,13 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) unsigned int slot_id = ep->vdev->slot_id; int err;
+ /* + * This is not going to work if the hardware is changing its dequeue + * pointers as we look at them. Completion handler will call us later. + */ + if (ep->ep_state & SET_DEQ_PENDING) + return 0; + xhci = ep->xhci;
list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) { @@ -1398,7 +1405,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, struct xhci_ep_ctx *ep_ctx; struct xhci_slot_ctx *slot_ctx; struct xhci_td *td, *tmp_td; - bool deferred = false;
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2])); @@ -1484,8 +1490,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, if (td->cancel_status == TD_CLEARING_CACHE) { td->cancel_status = TD_CLEARED; xhci_td_cleanup(ep->xhci, td, ep_ring, td->status); - } else if (td->cancel_status == TD_CLEARING_CACHE_DEFERRED) { - deferred = true; } } cleanup: @@ -1493,11 +1497,15 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, ep->queued_deq_seg = NULL; ep->queued_deq_ptr = NULL;
- if (deferred) { - /* We have more streams to clear */ + /* Check for deferred or newly cancelled TDs */ + if (!list_empty(&ep->cancelled_td_list)) { xhci_dbg(ep->xhci, "%s: Pending TDs to clear, continuing with invalidation\n", __func__); xhci_invalidate_cancelled_tds(ep); + /* Try to restart the endpoint if all is done */ + ring_doorbell_for_active_rings(xhci, slot_id, ep_index); + /* Start giving back any TDs invalidated above */ + xhci_giveback_invalidated_tds(ep); } else { /* Restart any rings with pending URBs */ xhci_dbg(ep->xhci, "%s: All TDs cleared, ring doorbell\n", __func__);
反馈： 您发送到kernel@openeuler.org的补丁/补丁集，已成功转换为PR！ PR链接地址： https://gitee.com/openeuler/kernel/pulls/14043 邮件列表地址：https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/7...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/14043 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/7...
-
patchwork bot -
Yongqiang Liu