From: Greg Kroah-Hartman Date: Tue, 3 Dec 2024 10:54:58 +0000 (+0100) Subject: 6.6-stable patches X-Git-Tag: v4.19.325~31 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=19664cdf42346b9124903519a710fa545605b26c;p=thirdparty%2Fkernel%2Fstable-queue.git 6.6-stable patches added patches: usb-dwc3-gadget-fix-checking-for-number-of-trbs-left.patch usb-dwc3-gadget-fix-looping-of-queued-sg-entries.patch usb-musb-fix-hardware-lockup-on-first-rx-endpoint-request.patch --- diff --git a/queue-6.6/series b/queue-6.6/series index 117dd3ed517..b528843aea0 100644 --- a/queue-6.6/series +++ b/queue-6.6/series @@ -551,3 +551,6 @@ smb-client-handle-max-length-for-smb-symlinks.patch smb-don-t-leak-cfid-when-reconnect-races-with-open_cached_dir.patch smb-prevent-use-after-free-due-to-open_cached_dir-error-paths.patch smb-during-unmount-ensure-all-cached-dir-instances-drop-their-dentry.patch +usb-musb-fix-hardware-lockup-on-first-rx-endpoint-request.patch +usb-dwc3-gadget-fix-checking-for-number-of-trbs-left.patch +usb-dwc3-gadget-fix-looping-of-queued-sg-entries.patch diff --git a/queue-6.6/usb-dwc3-gadget-fix-checking-for-number-of-trbs-left.patch b/queue-6.6/usb-dwc3-gadget-fix-checking-for-number-of-trbs-left.patch new file mode 100644 index 00000000000..f507aeff17e --- /dev/null +++ b/queue-6.6/usb-dwc3-gadget-fix-checking-for-number-of-trbs-left.patch @@ -0,0 +1,53 @@ +From 02a6982b0ccfcdc39e20016f5fc9a1b7826a6ee7 Mon Sep 17 00:00:00 2001 +From: Thinh Nguyen +Date: Thu, 14 Nov 2024 01:02:12 +0000 +Subject: usb: dwc3: gadget: Fix checking for number of TRBs left + +From: Thinh Nguyen + +commit 02a6982b0ccfcdc39e20016f5fc9a1b7826a6ee7 upstream. + +The check whether the TRB ring is full or empty in dwc3_calc_trbs_left() +is insufficient. It assumes there are active TRBs if there's any request +in the started_list. However, that's not the case for requests with a +large SG list. + +That is, if we have a single usb request that requires more TRBs than +the total TRBs in the TRB ring, the queued TRBs will be available when +all the TRBs in the ring are completed. But the request is only +partially completed and remains in the started_list. With the current +logic, the TRB ring is empty, but dwc3_calc_trbs_left() returns 0. + +Fix this by additionally checking for the request->num_trbs for active +TRB count. + +Cc: stable@vger.kernel.org +Fixes: 51f1954ad853 ("usb: dwc3: gadget: Fix dwc3_calc_trbs_left()") +Signed-off-by: Thinh Nguyen +Link: https://lore.kernel.org/r/708dc62b56b77da1f704cc2ae9b6ddb1f2dbef1f.1731545781.git.Thinh.Nguyen@synopsys.com +Signed-off-by: Greg Kroah-Hartman +Signed-off-by: Greg Kroah-Hartman +--- + drivers/usb/dwc3/gadget.c | 9 ++++++--- + 1 file changed, 6 insertions(+), 3 deletions(-) + +--- a/drivers/usb/dwc3/gadget.c ++++ b/drivers/usb/dwc3/gadget.c +@@ -1196,11 +1196,14 @@ static u32 dwc3_calc_trbs_left(struct dw + * pending to be processed by the driver. + */ + if (dep->trb_enqueue == dep->trb_dequeue) { ++ struct dwc3_request *req; ++ + /* +- * If there is any request remained in the started_list at +- * this point, that means there is no TRB available. ++ * If there is any request remained in the started_list with ++ * active TRBs at this point, then there is no TRB available. + */ +- if (!list_empty(&dep->started_list)) ++ req = next_request(&dep->started_list); ++ if (req && req->num_trbs) + return 0; + + return DWC3_TRB_NUM - 1; diff --git a/queue-6.6/usb-dwc3-gadget-fix-looping-of-queued-sg-entries.patch b/queue-6.6/usb-dwc3-gadget-fix-looping-of-queued-sg-entries.patch new file mode 100644 index 00000000000..b6bf2f83855 --- /dev/null +++ b/queue-6.6/usb-dwc3-gadget-fix-looping-of-queued-sg-entries.patch @@ -0,0 +1,49 @@ +From b7fc65f5141c24785dc8c19249ca4efcf71b3524 Mon Sep 17 00:00:00 2001 +From: Thinh Nguyen +Date: Thu, 14 Nov 2024 01:02:18 +0000 +Subject: usb: dwc3: gadget: Fix looping of queued SG entries + +From: Thinh Nguyen + +commit b7fc65f5141c24785dc8c19249ca4efcf71b3524 upstream. + +The dwc3_request->num_queued_sgs is decremented on completion. If a +partially completed request is handled, then the +dwc3_request->num_queued_sgs no longer reflects the total number of +num_queued_sgs (it would be cleared). + +Correctly check the number of request SG entries remained to be prepare +and queued. Failure to do this may cause null pointer dereference when +accessing non-existent SG entry. + +Cc: stable@vger.kernel.org +Fixes: c96e6725db9d ("usb: dwc3: gadget: Correct the logic for queuing sgs") +Signed-off-by: Thinh Nguyen +Link: https://lore.kernel.org/r/d07a7c4aa0fcf746cdca0515150dbe5c52000af7.1731545781.git.Thinh.Nguyen@synopsys.com +Signed-off-by: Greg Kroah-Hartman +--- + drivers/usb/dwc3/gadget.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +--- a/drivers/usb/dwc3/gadget.c ++++ b/drivers/usb/dwc3/gadget.c +@@ -1436,8 +1436,8 @@ static int dwc3_prepare_trbs_sg(struct d + struct scatterlist *s; + int i; + unsigned int length = req->request.length; +- unsigned int remaining = req->request.num_mapped_sgs +- - req->num_queued_sgs; ++ unsigned int remaining = req->num_pending_sgs; ++ unsigned int num_queued_sgs = req->request.num_mapped_sgs - remaining; + unsigned int num_trbs = req->num_trbs; + bool needs_extra_trb = dwc3_needs_extra_trb(dep, req); + +@@ -1445,7 +1445,7 @@ static int dwc3_prepare_trbs_sg(struct d + * If we resume preparing the request, then get the remaining length of + * the request and resume where we left off. + */ +- for_each_sg(req->request.sg, s, req->num_queued_sgs, i) ++ for_each_sg(req->request.sg, s, num_queued_sgs, i) + length -= sg_dma_len(s); + + for_each_sg(sg, s, remaining, i) { diff --git a/queue-6.6/usb-musb-fix-hardware-lockup-on-first-rx-endpoint-request.patch b/queue-6.6/usb-musb-fix-hardware-lockup-on-first-rx-endpoint-request.patch new file mode 100644 index 00000000000..7cc92d34672 --- /dev/null +++ b/queue-6.6/usb-musb-fix-hardware-lockup-on-first-rx-endpoint-request.patch @@ -0,0 +1,133 @@ +From 3fc137386c4620305bbc2a216868c53f9245670a Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Hubert=20Wi=C5=9Bniewski?= + +Date: Sun, 10 Nov 2024 18:21:48 +0100 +Subject: usb: musb: Fix hardware lockup on first Rx endpoint request +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Hubert Wiśniewski + +commit 3fc137386c4620305bbc2a216868c53f9245670a upstream. + +There is a possibility that a request's callback could be invoked from +usb_ep_queue() (call trace below, supplemented with missing calls): + +req->complete from usb_gadget_giveback_request + (drivers/usb/gadget/udc/core.c:999) +usb_gadget_giveback_request from musb_g_giveback + (drivers/usb/musb/musb_gadget.c:147) +musb_g_giveback from rxstate + (drivers/usb/musb/musb_gadget.c:784) +rxstate from musb_ep_restart + (drivers/usb/musb/musb_gadget.c:1169) +musb_ep_restart from musb_ep_restart_resume_work + (drivers/usb/musb/musb_gadget.c:1176) +musb_ep_restart_resume_work from musb_queue_resume_work + (drivers/usb/musb/musb_core.c:2279) +musb_queue_resume_work from musb_gadget_queue + (drivers/usb/musb/musb_gadget.c:1241) +musb_gadget_queue from usb_ep_queue + (drivers/usb/gadget/udc/core.c:300) + +According to the docstring of usb_ep_queue(), this should not happen: + +"Note that @req's ->complete() callback must never be called from within +usb_ep_queue() as that can create deadlock situations." + +In fact, a hardware lockup might occur in the following sequence: + +1. The gadget is initialized using musb_gadget_enable(). +2. Meanwhile, a packet arrives, and the RXPKTRDY flag is set, raising an + interrupt. +3. If IRQs are enabled, the interrupt is handled, but musb_g_rx() finds an + empty queue (next_request() returns NULL). The interrupt flag has + already been cleared by the glue layer handler, but the RXPKTRDY flag + remains set. +4. The first request is enqueued using usb_ep_queue(), leading to the call + of req->complete(), as shown in the call trace above. +5. If the callback enables IRQs and another packet is waiting, step (3) + repeats. The request queue is empty because usb_g_giveback() removes the + request before invoking the callback. +6. The endpoint remains locked up, as the interrupt triggered by hardware + setting the RXPKTRDY flag has been handled, but the flag itself remains + set. + +For this scenario to occur, it is only necessary for IRQs to be enabled at +some point during the complete callback. This happens with the USB Ethernet +gadget, whose rx_complete() callback calls netif_rx(). If called in the +task context, netif_rx() disables the bottom halves (BHs). When the BHs are +re-enabled, IRQs are also enabled to allow soft IRQs to be processed. The +gadget itself is initialized at module load (or at boot if built-in), but +the first request is enqueued when the network interface is brought up, +triggering rx_complete() in the task context via ioctl(). If a packet +arrives while the interface is down, it can prevent the interface from +receiving any further packets from the USB host. + +The situation is quite complicated with many parties involved. This +particular issue can be resolved in several possible ways: + +1. Ensure that callbacks never enable IRQs. This would be difficult to + enforce, as discovering how netif_rx() interacts with interrupts was + already quite challenging and u_ether is not the only function driver. + Similar "bugs" could be hidden in other drivers as well. +2. Disable MUSB interrupts in musb_g_giveback() before calling the callback + and re-enable them afterwars (by calling musb_{dis,en}able_interrupts(), + for example). This would ensure that MUSB interrupts are not handled + during the callback, even if IRQs are enabled. In fact, it would allow + IRQs to be enabled when releasing the lock. However, this feels like an + inelegant hack. +3. Modify the interrupt handler to clear the RXPKTRDY flag if the request + queue is empty. While this approach also feels like a hack, it wastes + CPU time by attempting to handle incoming packets when the software is + not ready to process them. +4. Flush the Rx FIFO instead of calling rxstate() in musb_ep_restart(). + This ensures that the hardware can receive packets when there is at + least one request in the queue. Once IRQs are enabled, the interrupt + handler will be able to correctly process the next incoming packet + (eventually calling rxstate()). This approach may cause one or two + packets to be dropped (two if double buffering is enabled), but this + seems to be a minor issue, as packet loss can occur when the software is + not yet ready to process them. Additionally, this solution makes the + gadget driver compliant with the rule mentioned in the docstring of + usb_ep_queue(). + +There may be additional solutions, but from these four, the last one has +been chosen as it seems to be the most appropriate, as it addresses the +"bad" behavior of the driver. + +Fixes: baebdf48c360 ("net: dev: Makes sure netif_rx() can be invoked in any context.") +Cc: stable@vger.kernel.org +Signed-off-by: Hubert Wiśniewski +Link: https://lore.kernel.org/r/4ee1ead4525f78fb5909a8cbf99513ad0082ad21.camel@gmail.com +Signed-off-by: Greg Kroah-Hartman +--- + drivers/usb/musb/musb_gadget.c | 13 ++++++++++--- + 1 file changed, 10 insertions(+), 3 deletions(-) + +--- a/drivers/usb/musb/musb_gadget.c ++++ b/drivers/usb/musb/musb_gadget.c +@@ -1170,12 +1170,19 @@ struct free_record { + */ + void musb_ep_restart(struct musb *musb, struct musb_request *req) + { ++ u16 csr; ++ void __iomem *epio = req->ep->hw_ep->regs; ++ + trace_musb_req_start(req); + musb_ep_select(musb->mregs, req->epnum); +- if (req->tx) ++ if (req->tx) { + txstate(musb, req); +- else +- rxstate(musb, req); ++ } else { ++ csr = musb_readw(epio, MUSB_RXCSR); ++ csr |= MUSB_RXCSR_FLUSHFIFO | MUSB_RXCSR_P_WZC_BITS; ++ musb_writew(epio, MUSB_RXCSR, csr); ++ musb_writew(epio, MUSB_RXCSR, csr); ++ } + } + + static int musb_ep_restart_resume_work(struct musb *musb, void *data)