All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.