Re: 答复: [外部邮件] Re: [PATCH] iommu/dma-iommu: Fix wrong scatterlist length assignment in P2PDMA path

[Logan Gunthorpe]

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@xxxxxxxxx>
>>
>> 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@xxxxxxxxxxxx>

Thanks!

Logan