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>
---
 drivers/iommu/dma-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 54d96e8..e8d4c2d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1465,7 +1465,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 			 */
 			s->dma_address = pci_p2pdma_bus_addr_map(
 				p2pdma_state.mem, sg_phys(s));
-			sg_dma_len(s) = sg->length;
+			sg_dma_len(s) = s->length;
 			sg_dma_mark_bus_address(s);
 			continue;
 		default:
-- 
2.9.4

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,

Logan

> 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

[Li,Rongqing] 

> Logan

Hi Li,

On 2026-06-02 00:12, Li,Rongqing wrote:
>>> 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.

Ok, yeah, I think you are correct. My tests that ran multiple segments
happened to all have the same length. But I don't have the hardware I
used at the moment so I can't test it either.

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

Thanks!

Logan

> 
> > 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