linux-kernel - [RFC PATCH 1/1] usb: dwc2: Don't set DEV_DMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230726102252.2236314-2-abailon@baylibre.com>
Date:   Wed, 26 Jul 2023 12:22:52 +0200
From:   Alexandre Bailon <abailon@...libre.com>
To:     hminas@...opsys.com
Cc:     gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Alexandre Bailon <abailon@...libre.com>
Subject: [RFC PATCH 1/1] usb: dwc2: Don't set DEV_DMA_L by default for isoc transfer

Currently, by default, we set DEV_DMA_L to notify DMA that the current
descriptor is the last one.
But, to get better performances, the driver doesn't stop the DMA and add
a new descriptor at the end of the list and clear DEV_DMA_L bit from
previous descriptor. This works well except if the DMA has already fetched
the descriptor. Updating the descriptor owned by DMA can cause some issues.

This updates the driver to no update descriptor while it is in use.
Instead, this assumes that we are always going to queue a request
and feed the DMA with new descriptors.
In case where we don't feed the DMA, we stop the transfer using a BNA.

NOTE:
To be tested!!!
Currently, only tested and validated using UAC2.

Signed-off-by: Alexandre Bailon <abailon@...libre.com>
---
 drivers/usb/dwc2/gadget.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 8b15742d9e8a..c542811468ce 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -928,10 +928,6 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 		return 1;
 	}
 
-	/* Clear L bit of previous desc if more than one entries in the chain */
-	if (hs_ep->next_desc)
-		hs_ep->desc_list[index - 1].status &= ~DEV_DMA_L;
-
 	dev_dbg(hsotg->dev, "%s: Filling ep %d, dir %s isoc desc # %d\n",
 		__func__, hs_ep->index, hs_ep->dir_in ? "in" : "out", index);
 
@@ -939,8 +935,9 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 	desc->status |= (DEV_DMA_BUFF_STS_HBUSY	<< DEV_DMA_BUFF_STS_SHIFT);
 
 	desc->buf = dma_buff;
-	desc->status |= (DEV_DMA_L | DEV_DMA_IOC |
-			 ((len << DEV_DMA_NBYTES_SHIFT) & mask));
+	desc->status |= (DEV_DMA_IOC | ((len << DEV_DMA_NBYTES_SHIFT) & mask));
+	if ((hs_ep->next_desc+1) >= MAX_DMA_DESC_NUM_HS_ISOC)
+		desc->status |= DEV_DMA_L;
 
 	if (hs_ep->dir_in) {
 		if (len)
@@ -968,6 +965,15 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 	if (hs_ep->next_desc >= MAX_DMA_DESC_NUM_HS_ISOC)
 		hs_ep->next_desc = 0;
 
+
+	/*
+	 * Safeguard: Make sure we stop transfer if we don't have a valid descriptor.
+	 */
+	index = hs_ep->next_desc;
+	desc = &hs_ep->desc_list[index];
+	desc->status = 0;
+	desc->status |= (DEV_DMA_BUFF_STS_HBUSY	<< DEV_DMA_BUFF_STS_SHIFT);
+
 	return 0;
 }
 
-- 
2.41.0