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;
}
next prev 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