[PATCHv3 1/2] nvme-pci: fix dma_vecs leak on p2p memory

[PATCHv3 1/2] nvme-pci: fix dma_vecs leak on p2p memory

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

We don't unmap P2P memory, so allocating the dma_vec for it was being
leaked on completion.

Fixes: b8b7570a7ec87 ("nvme-pci: fix dma unmapping when using PRPs and not using the IOVA mapping")
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 139a10cd687f9..744b388eea956 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -966,7 +966,8 @@ static bool nvme_pci_prp_save_mapping(struct request *req,
 {
 	struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
 
-	if (dma_use_iova(&iod->dma_state) || !dma_need_unmap(dma_dev))
+	if (dma_use_iova(&iod->dma_state) || !dma_need_unmap(dma_dev) ||
+	    iod->flags & IOD_DATA_P2P)
 		return true;
 
 	if (!iod->nr_dma_vecs) {
-- 
2.53.0-Meta

[PATCHv3 2/2] nvme-pci: fix dma mapping leak on data setup error

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

We're leaking the initial DMA mapping during iteration if we fail to
allocate the tracking descriptor for both PRP and SGL. Unmap the
iterator directly; we can't use the existing unmap helper because it
depends on the tracking descriptor being successfully allocated, so a
new one for an in-use iterator is provided.

The mappings were also leaking when the driver detects an invalid
bio_vec when mapping PRPs, so fix that too.

Fixes: b8b7570a7ec87 ("nvme-pci: fix dma unmapping when using PRPs and not using the IOVA mapping")
Fixes: 7ce3c1dd78fca ("nvme-pci: convert the data mapping to blk_rq_dma_map")
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v2->v3:

  Made the same fix for metadata sgl handling, and factored out a common
  helper that data and metadata can both use.

 drivers/nvme/host/pci.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 744b388eea956..d108dbe0d4ebd 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -997,6 +997,23 @@ static bool nvme_pci_prp_iter_next(struct request *req, struct device *dma_dev,
 	return nvme_pci_prp_save_mapping(req, dma_dev, iter);
 }
 
+static void nvme_unmap_iter(struct request *req, struct blk_dma_iter *iter,
+			    struct dma_iova_state *state)
+{
+	struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
+	struct device *dev = nvmeq->dev->dev;
+
+	if (!blk_rq_dma_unmap(req, dev, state, iter->len, iter->p2pdma.map)) {
+		unsigned int attrs = 0;
+
+		if (iter->p2pdma.map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
+			attrs |= DMA_ATTR_MMIO;
+
+		dma_unmap_phys(dev, iter->addr, iter->len, rq_dma_dir(req),
+			       attrs);
+	}
+}
+
 static blk_status_t nvme_pci_setup_data_prp(struct request *req,
 		struct blk_dma_iter *iter)
 {
@@ -1007,8 +1024,10 @@ static blk_status_t nvme_pci_setup_data_prp(struct request *req,
 	unsigned int prp_len, i;
 	__le64 *prp_list;
 
-	if (!nvme_pci_prp_save_mapping(req, nvmeq->dev->dev, iter))
+	if (!nvme_pci_prp_save_mapping(req, nvmeq->dev->dev, iter)) {
+		nvme_unmap_iter(req, iter, &iod->dma_state);
 		return iter->status;
+	}
 
 	/*
 	 * PRP1 always points to the start of the DMA transfers.
@@ -1113,6 +1132,7 @@ static blk_status_t nvme_pci_setup_data_prp(struct request *req,
 	dev_err_once(nvmeq->dev->dev,
 		"Incorrectly formed request for payload:%d nents:%d\n",
 		blk_rq_payload_bytes(req), blk_rq_nr_phys_segments(req));
+	nvme_unmap_data(req);
 	return BLK_STS_IOERR;
 }
 
@@ -1156,8 +1176,11 @@ static blk_status_t nvme_pci_setup_data_sgl(struct request *req,
 
 	sg_list = dma_pool_alloc(nvme_dma_pool(nvmeq, iod), GFP_ATOMIC,
 			&sgl_dma);
-	if (!sg_list)
+	if (!sg_list) {
+		nvme_unmap_iter(req, iter, &iod->dma_state);
 		return BLK_STS_RESOURCE;
+	}
+
 	iod->descriptors[iod->nr_descriptors++] = sg_list;
 
 	do {
@@ -1314,8 +1337,10 @@ static blk_status_t nvme_pci_setup_meta_iter(struct request *req)
 
 	sg_list = dma_pool_alloc(nvmeq->descriptor_pools.small, GFP_ATOMIC,
 			&sgl_dma);
-	if (!sg_list)
+	if (!sg_list) {
+		nvme_unmap_iter(req, &iter, &iod->meta_dma_state);
 		return BLK_STS_RESOURCE;
+	}
 
 	iod->meta_descriptor = sg_list;
 	iod->meta_dma = sgl_dma;
-- 
2.53.0-Meta

Re: [PATCHv3 1/2] nvme-pci: fix dma_vecs leak on p2p memory

[Christoph Hellwig]

On Wed, May 20, 2026 at 10:49:52AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> We don't unmap P2P memory, so allocating the dma_vec for it was being
> leaked on completion.
> 
> Fixes: b8b7570a7ec87 ("nvme-pci: fix dma unmapping when using PRPs and not using the IOVA mapping")
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/nvme/host/pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 139a10cd687f9..744b388eea956 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -966,7 +966,8 @@ static bool nvme_pci_prp_save_mapping(struct request *req,
>  {
>  	struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
>  
> -	if (dma_use_iova(&iod->dma_state) || !dma_need_unmap(dma_dev))
> +	if (dma_use_iova(&iod->dma_state) || !dma_need_unmap(dma_dev) ||
> +	    iod->flags & IOD_DATA_P2P)

Missing braces around the & for our usual style?

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCHv3 2/2] nvme-pci: fix dma mapping leak on data setup error

[Christoph Hellwig]

On Wed, May 20, 2026 at 10:49:53AM -0700, Keith Busch wrote:
> +static void nvme_unmap_iter(struct request *req, struct blk_dma_iter *iter,
> +			    struct dma_iova_state *state)
> +{
> +	struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
> +	struct device *dev = nvmeq->dev->dev;
> +
> +	if (!blk_rq_dma_unmap(req, dev, state, iter->len, iter->p2pdma.map)) {
> +		unsigned int attrs = 0;
> +
> +		if (iter->p2pdma.map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
> +			attrs |= DMA_ATTR_MMIO;
> +
> +		dma_unmap_phys(dev, iter->addr, iter->len, rq_dma_dir(req),
> +			       attrs);
> +	}

At some point this might be worth lifting to the block layer, but we
can defer that until needed.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCHv3 1/2] nvme-pci: fix dma_vecs leak on p2p memory

[Keith Busch]

On Thu, May 21, 2026 at 10:21:53AM +0200, Christoph Hellwig wrote:
> On Wed, May 20, 2026 at 10:49:52AM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > We don't unmap P2P memory, so allocating the dma_vec for it was being
> > leaked on completion.
> > 
> > Fixes: b8b7570a7ec87 ("nvme-pci: fix dma unmapping when using PRPs and not using the IOVA mapping")
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
> >  drivers/nvme/host/pci.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > index 139a10cd687f9..744b388eea956 100644
> > --- a/drivers/nvme/host/pci.c
> > +++ b/drivers/nvme/host/pci.c
> > @@ -966,7 +966,8 @@ static bool nvme_pci_prp_save_mapping(struct request *req,
> >  {
> >  	struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
> >  
> > -	if (dma_use_iova(&iod->dma_state) || !dma_need_unmap(dma_dev))
> > +	if (dma_use_iova(&iod->dma_state) || !dma_need_unmap(dma_dev) ||
> > +	    iod->flags & IOD_DATA_P2P)
> 
> Missing braces around the & for our usual style?

Thanks, fixed up and applied.