Linux IOMMU Development
 help / color / mirror / Atom feed
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] 

  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