> 
> > On 2026-05-30 05:28, lirongqing wrote:
> > > From: Li RongQing <lirongqing@baidu.com>
> > >
> > > In iommu_dma_map_sg(), when handling PCI P2PDMA cases, the DMA
> > length
> > > of the current scatterlist segment `s` is incorrectly assigned from
> > > the head entry `sg->length` instead of the current entry `s->length`.
> > >
> > > This typo causes all P2PDMA segments in the scatterlist to inherit
> > > the length of the first segment, leading to corrupted DMA lengths
> > > for
> > > multi- segment scatterlists.
> > >
> > > Fix this by using `s->length` instead of `sg->length`.
> > >
> > > Fixes: a25e7962db ("PCI/P2PDMA: Refactor the p2pdma mapping
> > > helpers")
> > > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> >
> > Hmm, I thought I had tested this stuff pretty thoroughly so I'm
> > surprised I missed that. But I guess seeing the users of these
> > specifically don't allow mixing with anonymous memory the vast
> > majority of tests would only have one segment.
> >
> > Can you confirm that you've reproduced and tested this and it actually
> > fixes a bug? Not just seeing something that looks strange. I have a
> > vague feeling it was intentional but looking at the code it looks wrong to
> me too.
> >
> > Thanks,
> >
> Cc: Christoph Hellwig
> 
> Thanks for your feedback!
> 
> This was found via code inspection. You are right that if nents == 1, or if
> nents > 1 but all segments happen to have the exact same length, this typo
> remains benign by pure coincidence.
> 
> However, for a multi-segment scatterlist with varying lengths (e.g., a short
> 2KB head segment followed by standard 4KB segments), sg_dma_len(s) will
> mistakenly assign the 2KB head length to all subsequent 4KB segments. This
> will inevitably lead to silent data truncation or IOMMU page faults.
> 
> Since sg_phys(s) correctly takes the current segment's address, sg_dma_len(s)
> should definitely use s->length to match it.
> 
> I don't have a specific P2PDMA test rig to force multi-segment lists on hand
> right now, but the typo and its impact on varied segment lengths seem
> deterministic.
> 
> Thanks
> 
AI report this commit a25e7962db0d79 ("PCI/P2PDMA: Refactor the p2pdma mapping helpers") has other issue, sg->dma_address is uninitialized in case of PCI_P2PDMA_MAP_THRU_HOST_BRIDGE in kernel/dma/direct.c, a possible fix is below:

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 583c592..4391b79 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -476,7 +476,7 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
                         * must be mapped with CPU physical address and not PCI
                         * bus addresses.
                         */
-                       break;
+                       fallthrough;
                case PCI_P2PDMA_MAP_NONE:
                        need_sync = true;
                        sg->dma_address = dma_direct_map_phys(dev, sg_phys(sg),


thanks

[Li,Rongqing] 

On 2026-06-02 03:48, Li,Rongqing wrote:
> AI report this commit a25e7962db0d79 ("PCI/P2PDMA: Refactor the p2pdma mapping helpers") has other issue, sg->dma_address is uninitialized in case of PCI_P2PDMA_MAP_THRU_HOST_BRIDGE in kernel/dma/direct.c, a possible fix is below:
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 583c592..4391b79 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -476,7 +476,7 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
>                          * must be mapped with CPU physical address and not PCI
>                          * bus addresses.
>                          */
> -                       break;
> +                       fallthrough;
>                 case PCI_P2PDMA_MAP_NONE:
>                         need_sync = true;
>                         sg->dma_address = dma_direct_map_phys(dev, sg_phys(sg),
> 

Yes, this looks correct as well. Not sure how we missed that.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Thanks,

Logan