2024-05-23 10:05:39

by Peng Hongchi

[permalink] [raw]
Subject: [PATCH] usb: dwc2: gadget: Don't write invalid mapped sg entries into dma_desc with iommu enabled

When using dma_map_sg() to map the scatterlist with iommu enabled,
the entries in the scatterlist can be mergerd into less but longer
entries in the function __finalise_sg(). So that the number of
valid mapped entries is actually smaller than ureq->num_reqs,and
there are still some invalid entries in the scatterlist with
dma_addr=0xffffffff and len=0. Writing these invalid sg entries
into the dma_desc can cause a data transmission error.

The function dma_map_sg() returns the number of valid map entries
and the return value is assigned to usb_request::num_mapped_sgs in
function usb_gadget_map_request_by_dev(). So that just write valid
mapped entries into dma_desc according to the usb_request::num_mapped_sgs,
and set the IOC bit if it's the last valid mapped entry.

This patch poses no risk to no-iommu situation, cause
ureq->num_mapped_sgs equals ureq->num_sgs while using dma_direct_map_sg()
to map the scatterlist whith iommu disabled.

Signed-off-by: Peng Hongchi <[email protected]>
---
drivers/usb/dwc2/gadget.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 74ac79abd8f3..e7bf9cc635be 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -885,10 +885,10 @@ static void dwc2_gadget_config_nonisoc_xfer_ddma(struct dwc2_hsotg_ep *hs_ep,
}

/* DMA sg buffer */
- for_each_sg(ureq->sg, sg, ureq->num_sgs, i) {
+ for_each_sg(ureq->sg, sg, ureq->num_mapped_sgs, i) {
dwc2_gadget_fill_nonisoc_xfer_ddma_one(hs_ep, &desc,
sg_dma_address(sg) + sg->offset, sg_dma_len(sg),
- sg_is_last(sg));
+ (i == (ureq->num_mapped_sgs - 1)));
desc_count += hs_ep->desc_count;
}

--
2.34.1



2024-06-06 07:12:46

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: gadget: Don't write invalid mapped sg entries into dma_desc with iommu enabled



On 5/23/24 14:03, Peng Hongchi wrote:
> When using dma_map_sg() to map the scatterlist with iommu enabled,
> the entries in the scatterlist can be mergerd into less but longer
> entries in the function __finalise_sg(). So that the number of
> valid mapped entries is actually smaller than ureq->num_reqs,and
> there are still some invalid entries in the scatterlist with
> dma_addr=0xffffffff and len=0. Writing these invalid sg entries
> into the dma_desc can cause a data transmission error.
>
> The function dma_map_sg() returns the number of valid map entries
> and the return value is assigned to usb_request::num_mapped_sgs in
> function usb_gadget_map_request_by_dev(). So that just write valid
> mapped entries into dma_desc according to the usb_request::num_mapped_sgs,
> and set the IOC bit if it's the last valid mapped entry.
>
> This patch poses no risk to no-iommu situation, cause
> ureq->num_mapped_sgs equals ureq->num_sgs while using dma_direct_map_sg()
> to map the scatterlist whith iommu disabled.
>
> Signed-off-by: Peng Hongchi <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> drivers/usb/dwc2/gadget.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 74ac79abd8f3..e7bf9cc635be 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -885,10 +885,10 @@ static void dwc2_gadget_config_nonisoc_xfer_ddma(struct dwc2_hsotg_ep *hs_ep,
> }
>
> /* DMA sg buffer */
> - for_each_sg(ureq->sg, sg, ureq->num_sgs, i) {
> + for_each_sg(ureq->sg, sg, ureq->num_mapped_sgs, i) {
> dwc2_gadget_fill_nonisoc_xfer_ddma_one(hs_ep, &desc,
> sg_dma_address(sg) + sg->offset, sg_dma_len(sg),
> - sg_is_last(sg));
> + (i == (ureq->num_mapped_sgs - 1)));
> desc_count += hs_ep->desc_count;
> }
>