public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Alex Mastro <amastro@fb.com>
To: Alex Williamson <alex@shazbot.org>
Cc: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>,
	Jason Gunthorpe <jgg@ziepe.ca>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	David Matlack <dmatlack@google.com>
Subject: Re: [PATCH v4 0/3] vfio: handle DMA map/unmap up to the addressable limit
Date: Thu, 23 Oct 2025 13:52:04 -0700	[thread overview]
Message-ID: <aPqVdAB9F9Go5X3n@devgpu012.nha5.facebook.com> (raw)
In-Reply-To: <aPe0E6Jj9BJA2Bd5@devgpu012.nha5.facebook.com>

One point to clarify

> On Mon, Oct 20, 2025 at 03:36:33PM -0600, Alex Williamson wrote:
> > Along with the tag, it would probably be useful in that same commit to
> > expand on the scope of the issue in the commit log.  I believe we allow
> > mappings to be created at the top of the address space that cannot be
> > removed via ioctl,

True

> > but such inconsistency should result in an application error due to the
> > failed ioctl

Actually, the ioctl does not fail in the sense that the caller gets an errno.
Attempting to unmap a range ending at the end of address space succeeds (returns
zero), but unmaps zero bytes. An application would only detect this if it
explicitly checked the out size field of vfio_iommu_type1_dma_unmap. Or
attempted to create another overlapping mapping on top of the ranges it expected
to be unmapped.

Annotated below:

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 916cad80941c..039cba5a38ca 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -165,19 +165,27 @@ vfio_iommu_find_iommu_group(struct vfio_iommu *iommu,
 static struct vfio_dma *vfio_find_dma(struct vfio_iommu *iommu,
 				      dma_addr_t start, size_t size)
 {
+	// start == ~(dma_addr_t)0
+	// size == 0
 	struct rb_node *node = iommu->dma_list.rb_node;
 
 	while (node) {
 		struct vfio_dma *dma = rb_entry(node, struct vfio_dma, node);
 
+		// never true because all dma->iova < ~(dma_addr_t)0
 		if (start + size <= dma->iova)
 			node = node->rb_left;
+		// traverses right until we get to the end of address space
+		// dma, then we walk off the end because
+		// ~(dma_addr_t)0 >= 0 == true
+		// node = NULL
 		else if (start >= dma->iova + dma->size)
 			node = node->rb_right;
 		else
 			return dma;
 	}
 
+	// This happens
 	return NULL;
 }
 
@@ -201,6 +209,8 @@ static struct rb_node *vfio_find_dma_first_node(struct vfio_iommu *iommu,
 			node = node->rb_right;
 		}
 	}
+	// iova >= start + size == true, due to overflow to zero => no first
+	// node found
 	if (res && size && dma_res->iova >= start + size)
 		res = NULL;
 	return res;
@@ -1397,6 +1407,7 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
 		if (iova || size)
 			goto unlock;
 		size = U64_MAX;
+	// end of address space unmap passes these checks
 	} else if (!size || size & (pgsize - 1) ||
 		   iova + size - 1 < iova || size > SIZE_MAX) {
 		goto unlock;
@@ -1442,18 +1453,23 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
 	 * mappings within the range.
 	 */
 	if (iommu->v2 && !unmap_all) {
+		// passes this check as long as the unmap start doesn't split an
+		// existing dma
 		dma = vfio_find_dma(iommu, iova, 1);
 		if (dma && dma->iova != iova)
 			goto unlock;
 
+		// dma = NULL, we pass this check
 		dma = vfio_find_dma(iommu, iova + size - 1, 0);
 		if (dma && dma->iova + dma->size != iova + size)
 			goto unlock;
 	}
 
 	ret = 0;
+	// n = NULL
 	n = first_n = vfio_find_dma_first_node(iommu, iova, size);
 
+	// loop body never entered
 	while (n) {
 		dma = rb_entry(n, struct vfio_dma, node);
 		if (dma->iova >= iova + size)
@@ -1513,6 +1529,7 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
 	/* Report how much was unmapped */
 	unmap->size = unmapped;
 
+	// return 0
 	return ret;
 }
 

  parent reply	other threads:[~2025-10-23 20:52 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13  5:32 [PATCH v4 0/3] vfio: handle DMA map/unmap up to the addressable limit Alex Mastro
2025-10-13  5:32 ` [PATCH v4 1/3] vfio/type1: sanitize for overflow using check_*_overflow Alex Mastro
2025-10-13  5:32 ` [PATCH v4 2/3] vfio/type1: move iova increment to unmap_unpin_* caller Alex Mastro
2025-10-13  5:32 ` [PATCH v4 3/3] vfio/type1: handle DMA map/unmap up to the addressable limit Alex Mastro
2025-10-21 22:18   ` Alejandro Jimenez
2025-10-22 14:24     ` Alex Mastro
2025-10-15 19:24 ` [PATCH v4 0/3] vfio: " Alex Williamson
2025-10-15 21:25   ` Alejandro Jimenez
2025-10-16 21:19     ` Alex Mastro
2025-10-16 22:01       ` Alex Williamson
2025-10-17 16:29         ` Alex Mastro
2025-10-20 21:36           ` Alex Williamson
2025-10-21 16:25             ` Alex Mastro
2025-10-21 16:31               ` David Matlack
2025-10-21 19:13                 ` Alex Mastro
2025-10-22  0:38                   ` David Matlack
2025-10-22 14:55                     ` Alex Mastro
2025-10-23 20:52               ` Alex Mastro [this message]
2025-10-23 22:33                 ` Alex Williamson
2025-10-27 16:02               ` Alex Mastro
2025-10-28  1:57                 ` Alex Williamson
2025-10-28 15:29                   ` Alex Mastro
2025-10-21 12:36 ` Jason Gunthorpe
2025-10-21 22:21 ` Alejandro Jimenez
2025-10-25 18:11 ` Alex Mastro
2025-10-27 13:39   ` Jason Gunthorpe
2025-10-28 18:42     ` Alex Mastro

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=aPqVdAB9F9Go5X3n@devgpu012.nha5.facebook.com \
    --to=amastro@fb.com \
    --cc=alejandro.j.jimenez@oracle.com \
    --cc=alex@shazbot.org \
    --cc=dmatlack@google.com \
    --cc=jgg@ziepe.ca \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.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