From: "Li,Rongqing" <lirongqing@baidu.com>
To: Logan Gunthorpe <logang@deltatee.com>,
Robin Murphy <robin.murphy@arm.com>,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
Lu Baolu <baolu.lu@linux.intel.com>,
Luis Chamberlain <mcgrof@kernel.org>,
Leon Romanovsky <leon@kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>,
"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "hch@lst.de" <hch@lst.de>
Subject: 答复: [外部邮件] Re: [PATCH] iommu/dma-iommu: Fix wrong scatterlist length assignment in P2PDMA path
Date: Tue, 2 Jun 2026 09:48:44 +0000 [thread overview]
Message-ID: <ceebcd436ac44548a299ed7b44cee4c8@baidu.com> (raw)
In-Reply-To: <5e2f30a4121e4672ab309460630d011b@baidu.com>
>
> > 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]
next prev parent reply other threads:[~2026-06-02 9:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-30 11:28 [PATCH] iommu/dma-iommu: Fix wrong scatterlist length assignment in P2PDMA path lirongqing
2026-06-01 16:02 ` Logan Gunthorpe
2026-06-02 6:12 ` 答复: [外部邮件] " Li,Rongqing
2026-06-02 9:48 ` Li,Rongqing [this message]
2026-06-02 15:51 ` Logan Gunthorpe
2026-06-02 15:42 ` Logan Gunthorpe
2026-06-11 11:59 ` Jason Gunthorpe
2026-06-11 12:21 ` 答复: " Li,Rongqing
2026-06-12 13:23 ` Joerg Roedel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ceebcd436ac44548a299ed7b44cee4c8@baidu.com \
--to=lirongqing@baidu.com \
--cc=baolu.lu@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=hch@lst.de \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=logang@deltatee.com \
--cc=mcgrof@kernel.org \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox