[PATCH 0/4] iommufd: Keep IOMMU_HWPT_GET_DIRTY_BITMAP within the user bitmap

[PATCH 0/4] iommufd: Keep IOMMU_HWPT_GET_DIRTY_BITMAP within the user bitmap

[Andrea Parri]

IOMMU_HWPT_GET_DIRTY_BITMAP can set bits past the end of the bitmap the
caller passed in. Two cases, both fixed here.

iova_bitmap_set() does not clip the range it records to the bitmap. When
a query ends inside a dirty huge IOPTE, the generic page table code
records the whole entry and the bits past the end of the user bitmap are
set, up to the end of the last pinned page. The helper is shared with
VFIO DMA logging, so the clip covers both users. Patch 1 fixes it and
patch 2 tests it.

iommufd_check_iova_range() subtracts one from the length before checking
it, so iova = 0 with length = 0 is accepted and reaches
iova_bitmap_alloc() with length - 1 == SIZE_MAX, a range the clip in
patch 1 cannot bound. Patch 3 rejects a zero length and patch 4 tests
it.

Tested on x86-64 under virtme-ng with CONFIG_IOMMUFD_TEST=y and
hugepages=128. Without the fixes the new tests fail: the overrun test
finds 56 bytes set past an 8-byte bitmap, and the zero-length query
returns success. With the series the iommufd_dirty_tracking fixture
passes, the overrun test skipping on the variants without huge IOPTEs.

Andrea Parri (4):
  iommufd/iova_bitmap: Clip recorded ranges to the bitmap
  iommufd/selftest: Test dirty bitmap recording past the query
  iommufd: Reject a zero-length dirty bitmap request
  iommufd/selftest: Test a zero-length dirty bitmap request

 drivers/iommu/iommufd/io_pagetable.c    |  3 ++
 drivers/iommu/iommufd/iova_bitmap.c     | 27 ++++++++++
 tools/testing/selftests/iommu/iommufd.c | 68 +++++++++++++++++++++++++
 3 files changed, 98 insertions(+)


base-commit: 54dadb030c7e2350957855d3995de05ae02c2e66
-- 
2.53.0

[PATCH 1/4] iommufd/iova_bitmap: Clip recorded ranges to the bitmap

[Andrea Parri]

IOMMU_HWPT_GET_DIRTY_BITMAP can set bits in the caller's memory past the
end of the bitmap.  When the queried range ends inside a dirty huge
IOPTE, the generic page-table dirty code records the whole entry, and
iova_bitmap_set() then sets every bit up to the end of the pinned page.

iova_bitmap_mapped_range() checks a range only against the pages
currently pinned, and iova_bitmap_mapped_length() caps only the pinning
window, so neither limits the write.  A range that starts inside the
bitmap but ends past it passes iova_bitmap_advance_to() on its start,
and the write loop runs to the end of the last pinned page.

Clip the range to [bitmap->iova, bitmap->iova + bitmap->length - 1]
before computing cur_bit and last_bit.  A range that starts before the
bitmap now has its in-range part recorded; the unsigned underflow in
iova_bitmap_advance_to() previously rejected it whole.

The clip runs before any pinning or indexing, so for a validated query
range cur_bit and last_bit can no longer address outside the bitmap.  It
only narrows the requested range; the bitmap bounds are fixed at
allocation.

Fixes: 58ccf0190d19 ("vfio: Add an IOVA bitmap support")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 drivers/iommu/iommufd/iova_bitmap.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/iommu/iommufd/iova_bitmap.c b/drivers/iommu/iommufd/iova_bitmap.c
index dac3e657d498d..44826ae330c4b 100644
--- a/drivers/iommu/iommufd/iova_bitmap.c
+++ b/drivers/iommu/iommufd/iova_bitmap.c
@@ -388,6 +388,30 @@ int iova_bitmap_for_each(struct iova_bitmap *bitmap, void *opaque,
 }
 EXPORT_SYMBOL_NS_GPL(iova_bitmap_for_each, "IOMMUFD");
 
+/*
+ * Trim [@iova..@iova+@length-1] to the IOVA range covered by @bitmap.
+ * Dirty trackers may report more than was asked for, for instance a whole
+ * huge IOPTE of which only a part was queried.
+ */
+static bool iova_bitmap_clip(struct iova_bitmap *bitmap, unsigned long *iova,
+			     size_t *length)
+{
+	unsigned long first = max(*iova, bitmap->iova);
+	unsigned long last = bitmap->iova + bitmap->length - 1;
+	unsigned long end;
+
+	if (!*length)
+		return false;
+	if (!check_add_overflow(*iova, *length - 1, &end))
+		last = min(last, end);
+	if (first > last)
+		return false;
+
+	*iova = first;
+	*length = last - first + 1;
+	return true;
+}
+
 /**
  * iova_bitmap_set() - Records an IOVA range in bitmap
  * @bitmap: IOVA bitmap
@@ -404,6 +428,9 @@ void iova_bitmap_set(struct iova_bitmap *bitmap,
 	struct iova_bitmap_map *mapped = &bitmap->mapped;
 	unsigned long cur_bit, last_bit, last_page_idx;
 
+	if (!iova_bitmap_clip(bitmap, &iova, &length))
+		return;
+
 update_indexes:
 	if (unlikely(!iova_bitmap_mapped_range(mapped, iova, length))) {
 		/*
-- 
2.53.0

[PATCH 2/4] iommufd/selftest: Test dirty bitmap recording past the query

[Andrea Parri]

Add a dirty-tracking test that dirties a huge IOPTE and queries only part
of it, with a guard placed after an exactly sized bitmap.

The test maps hugetlb-backed memory, allocates a MOCK_IOMMUPT_HUGE
dirty-tracking HWPT, and dirties the first 1M IOPTE.  It then queries a
128K window at the mock 2K page size, so the request covers 64 bits while
the tracker records 512.  A guard follows the one-u64 bitmap in the same
pinned page.

Without the clip in iova_bitmap_set(), the kernel sets 56 bytes past the
bitmap and the guard check fails:

  # iommufd.c:2470:get_dirty_bitmap_overrun:Expected 0xaaaaaaaaaaaaaaaaULL (12297829382473034410) == bitmap[bitmap_words + i] (18446744073709551615)
  #          FAIL  iommufd_dirty_tracking.domain_dirty64M_huge.get_dirty_bitmap_overrun

With the fix the guard stays intact.  The test runs only on the *_huge
variants and skips the rest.  Tested on x86-64 under virtme-ng with
CONFIG_IOMMUFD_TEST=y and hugepages=128 on the kernel command line; the
whole iommufd_dirty_tracking fixture reports PASSED: 63/63, the seven
skips being this test on the non-huge variants.

Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 tools/testing/selftests/iommu/iommufd.c | 49 +++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index 44193d171ba98..47d861b3bdd7b 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -2423,6 +2423,55 @@ TEST_F(iommufd_dirty_tracking, get_dirty_bitmap_no_clear)
 	test_ioctl_destroy(hwpt_id);
 }
 
+TEST_F(iommufd_dirty_tracking, get_dirty_bitmap_overrun)
+{
+	uint32_t ioas_id = self->ioas_id;
+	uint32_t hwpt_id;
+	size_t nbits = 64;
+	size_t query_len = nbits * self->page_size;
+	__u64 *bitmap = self->bitmap;
+	size_t bitmap_words =
+		DIV_ROUND_UP(nbits, BITS_PER_BYTE * sizeof(*bitmap));
+	__u64 set = 1;
+	__u64 nr_dirty;
+	size_t i;
+
+	if (!variant->hugepages)
+		SKIP(return, "test requires a huge IOPTE");
+
+	/* The canary sits in the same pinned page, right after the bitmap. */
+	memset(bitmap, 0, (bitmap_words + 8) * sizeof(*bitmap));
+	for (i = 0; i < 8; i++)
+		bitmap[bitmap_words + i] = 0xaaaaaaaaaaaaaaaaULL;
+
+	test_ioctl_ioas_map_fixed_id(ioas_id, self->buffer,
+				     variant->buffer_size, MOCK_APERTURE_START);
+	test_cmd_hwpt_alloc_iommupt(self->idev_id, ioas_id,
+				    IOMMU_HWPT_ALLOC_DIRTY_TRACKING,
+				    MOCK_IOMMUPT_HUGE, &hwpt_id);
+	test_cmd_set_dirty_tracking(hwpt_id, true);
+
+	/* Dirty the whole first huge IOPTE */
+	test_cmd_mock_domain_set_dirty(self->fd, hwpt_id, MOCK_HUGE_PAGE_SIZE,
+				       MOCK_APERTURE_START, MOCK_HUGE_PAGE_SIZE,
+				       &set, &nr_dirty);
+	ASSERT_EQ(1, nr_dirty);
+
+	/* Query only part of it */
+	test_cmd_get_dirty_bitmap(self->fd, hwpt_id, query_len,
+				  MOCK_APERTURE_START, self->page_size, bitmap,
+				  0);
+
+	/* The queried window is dirty */
+	ASSERT_EQ(~0ULL, bitmap[0]);
+
+	/* Bits past the queried bitmap must not be set */
+	for (i = 0; i < 8; i++)
+		ASSERT_EQ(0xaaaaaaaaaaaaaaaaULL, bitmap[bitmap_words + i]);
+
+	test_ioctl_destroy(hwpt_id);
+}
+
 /* VFIO compatibility IOCTLs */
 
 TEST_F(iommufd, simple_ioctls)
-- 
2.53.0

[PATCH 3/4] iommufd: Reject a zero-length dirty bitmap request

[Andrea Parri]

iommufd_check_iova_range() subtracts one from bitmap->length before it
checks the length, so a zero-length request wraps to SIZE_MAX.  With
iova = 0 the wrapped value passes the overflow and alignment checks, and
the request reaches iova_bitmap_alloc() with length 0.

iova_bitmap_alloc() then sizes the bitmap from a length - 1 of SIZE_MAX,
so mapped_total_index is effectively unbounded.  The scan uses
last_iova = ULONG_MAX, which spans the whole IOAS when an area is mapped
at IOVA 0.  Reject a zero length before the subtraction.

Fixes: b9a60d6f850e ("iommufd: Add IOMMU_HWPT_GET_DIRTY_BITMAP")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 drivers/iommu/iommufd/io_pagetable.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/iommufd/io_pagetable.c b/drivers/iommu/iommufd/io_pagetable.c
index 4e447ce74cf6c..283ab372e8da8 100644
--- a/drivers/iommu/iommufd/io_pagetable.c
+++ b/drivers/iommu/iommufd/io_pagetable.c
@@ -605,6 +605,9 @@ int iommufd_check_iova_range(struct io_pagetable *iopt,
 	size_t iommu_pgsize = iopt->iova_alignment;
 	u64 last_iova;
 
+	if (!bitmap->length)
+		return -EINVAL;
+
 	if (check_add_overflow(bitmap->iova, bitmap->length - 1, &last_iova))
 		return -EOVERFLOW;
 
-- 
2.53.0

[PATCH 4/4] iommufd/selftest: Test a zero-length dirty bitmap request

[Andrea Parri]

Add a GET_DIRTY_BITMAP case that passes iova = 0 with a zero length and
expects -EINVAL. iova = 0 is the value that lets the zero length wrap
into an accepted range; without the previous patch the request is
accepted and returns success.

The case runs on every variant of the dirty-tracking fixture. Tested on
x86-64 under virtme-ng with CONFIG_IOMMUFD_TEST=y.

Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 tools/testing/selftests/iommu/iommufd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index 47d861b3bdd7b..2b3c9e9c69ca7 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -2472,6 +2472,25 @@ TEST_F(iommufd_dirty_tracking, get_dirty_bitmap_overrun)
 	test_ioctl_destroy(hwpt_id);
 }
 
+TEST_F(iommufd_dirty_tracking, get_dirty_bitmap_zero_length)
+{
+	uint32_t ioas_id = self->ioas_id;
+	uint32_t hwpt_id;
+
+	test_cmd_hwpt_alloc_iommupt(self->idev_id, ioas_id,
+				    IOMMU_HWPT_ALLOC_DIRTY_TRACKING,
+				    MOCK_IOMMUPT_DEFAULT, &hwpt_id);
+	test_cmd_set_dirty_tracking(hwpt_id, true);
+
+	/* Only iova 0 wraps the zero length into an accepted range. */
+	EXPECT_ERRNO(EINVAL,
+		     _test_cmd_get_dirty_bitmap(self->fd, hwpt_id, 0, 0,
+						self->page_size, self->bitmap,
+						0));
+
+	test_ioctl_destroy(hwpt_id);
+}
+
 /* VFIO compatibility IOCTLs */
 
 TEST_F(iommufd, simple_ioctls)
-- 
2.53.0

Re: [PATCH 1/4] iommufd/iova_bitmap: Clip recorded ranges to the bitmap

[Jason Gunthorpe]

On Fri, Sep 25, 2026 at 06:11:37PM +0200, Andrea Parri wrote:
> IOMMU_HWPT_GET_DIRTY_BITMAP can set bits in the caller's memory past the
> end of the bitmap.  When the queried range ends inside a dirty huge
> IOPTE, the generic page-table dirty code records the whole entry, and
> iova_bitmap_set() then sets every bit up to the end of the pinned page.
> 
> iova_bitmap_mapped_range() checks a range only against the pages
> currently pinned, and iova_bitmap_mapped_length() caps only the pinning
> window, so neither limits the write.  A range that starts inside the
> bitmap but ends past it passes iova_bitmap_advance_to() on its start,
> and the write loop runs to the end of the last pinned page.
> 
> Clip the range to [bitmap->iova, bitmap->iova + bitmap->length - 1]
> before computing cur_bit and last_bit.  A range that starts before the
> bitmap now has its in-range part recorded; the unsigned underflow in
> iova_bitmap_advance_to() previously rejected it whole.
> 
> The clip runs before any pinning or indexing, so for a validated query
> range cur_bit and last_bit can no longer address outside the bitmap.  It
> only narrows the requested range; the bitmap bounds are fixed at
> allocation.
> 
> Fixes: 58ccf0190d19 ("vfio: Add an IOVA bitmap support")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
>  drivers/iommu/iommufd/iova_bitmap.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/iommu/iommufd/iova_bitmap.c b/drivers/iommu/iommufd/iova_bitmap.c
> index dac3e657d498d..44826ae330c4b 100644
> --- a/drivers/iommu/iommufd/iova_bitmap.c
> +++ b/drivers/iommu/iommufd/iova_bitmap.c
> @@ -388,6 +388,30 @@ int iova_bitmap_for_each(struct iova_bitmap *bitmap, void *opaque,
>  }
>  EXPORT_SYMBOL_NS_GPL(iova_bitmap_for_each, "IOMMUFD");
>  
> +/*
> + * Trim [@iova..@iova+@length-1] to the IOVA range covered by @bitmap.
> + * Dirty trackers may report more than was asked for, for instance a whole
> + * huge IOPTE of which only a part was queried.
> + */
> +static bool iova_bitmap_clip(struct iova_bitmap *bitmap, unsigned long *iova,
> +			     size_t *length)
> +{
> +	unsigned long first = max(*iova, bitmap->iova);
> +	unsigned long last = bitmap->iova + bitmap->length - 1;
> +	unsigned long end;
> +
> +	if (!*length)
> +		return false;
> +	if (!check_add_overflow(*iova, *length - 1, &end))
> +		last = min(last, end);
> +	if (first > last)
> +		return false;
> +
> +	*iova = first;
> +	*length = last - first + 1;
> +	return true;
> +}
> +
>  /**
>   * iova_bitmap_set() - Records an IOVA range in bitmap
>   * @bitmap: IOVA bitmap
> @@ -404,6 +428,9 @@ void iova_bitmap_set(struct iova_bitmap *bitmap,
>  	struct iova_bitmap_map *mapped = &bitmap->mapped;
>  	unsigned long cur_bit, last_bit, last_page_idx;
>  
> +	if (!iova_bitmap_clip(bitmap, &iova, &length))
> +		return;

There have been several patches about the dirty stuff all together and
comprehensively this is not quite the right solution.

If the caller is providing a dirty range that exceeds the bitmap then
this is coming from something like a large PTE being dirty.

If we silently truncate that to the range of the bitmap, like this
does, then we also are silently lossing dirty marks. The userspace
will call again with an iova range stepped ahead and instead of seeing
the early blocks being dirty they will be marked as clean.

There are several bugs along these lines here as generic_pt is also
incorrectly clamping the ranges of the PTE.

So.. The right answer is for this to propogate an error when it
fails. There is another patch that provides the mechanism for this
already.

I will change this to just be:

@@ -409,6 +409,18 @@ void iova_bitmap_set(struct iova_bitmap *bitmap,
        struct iova_bitmap_map *mapped = &bitmap->mapped;
        unsigned long cur_bit, last_bit, last_page_idx;
 
+       /*
+        * If the range cannot be recorded then the system call returns failure.
+        * This is to prevent loosing dirties if userspace is expecting to step
+        * the bitmap through the IOVA space.
+        */
+       if (!length || length > bitmap->length || iova < bitmap->iova ||
+           iova - bitmap->iova > bitmap->length - length) {
+               if (!bitmap->err)
+                       bitmap->err = -EPROTO;
+               return;
+       }
+

Jason

Re: [PATCH 1/4] iommufd/iova_bitmap: Clip recorded ranges to the bitmap

[Andrea Parri]

On Wed, Oct 14, 2026 at 09:26:47PM -0300, Jason Gunthorpe wrote:
> If we silently truncate that to the range of the bitmap, like this
> does, then we also are silently lossing dirty marks. The userspace
> will call again with an iova range stepped ahead and instead of seeing
> the early blocks being dirty they will be marked as clean.

Right, the clip turns an overrun into silent dirty loss; failing the
call is better.

I also saw your reply to Samarth's record_dirty() patch [1]: with both
changes, a dirty entry that is not wholly inside the bitmap fails the
call.  Samarth offered to send a v2 of both patches; I'm happy to leave
1/4 to whichever of you prefers and rebase my tests on top.

A few things I'd like to check before I rework the rest:

1) The dirty bit is still cleared when the record fails.  In the
   record_dirty() you proposed, as in visit_dirty() in io-pgtable-arm,
   the entry is made clean unless IOMMU_DIRTY_NO_CLEAR, whatever
   iova_bitmap_set() did.  With start_va rounded down to the entry, a
   query that starts or ends inside a dirty huge IOPTE now fails this
   way too, and a retry with an aligned range sees the entry clean.  Is
   -EPROTO meant to be fatal for userspace (e.g. abort the migration),
   or should the clear be skipped once bitmap->err is set?

2) Could you point me at the patch that adds bitmap->err?  On mainline
   iommu_read_and_clear_dirty() ignores the return of
   iova_bitmap_for_each() and returns 0, so I assume that patch also
   changes this.  Does it cover the VFIO DMA logging path as well
   (vfio_device_log_read_and_clear(), with mlx5 and pds calling
   iova_bitmap_set())?

3) Patch 2 tests the clip, so with your change it fails: the 1M IOPTE
   at MOCK_APERTURE_START is rejected whole against the 128K query, and
   bitmap[0] stays clear.  I can respin it to expect EPROTO, keep the
   guard check past the bitmap, and add a query that starts inside the
   IOPTE, on top of the bitmap->err patch.

Patches 3 and 4 still apply, I think: iova = 0 with length = 0 passes
iommufd_check_iova_range(), and __iommu_read_and_clear_dirty() then
walks [0, ULONG_MAX].  With your check nothing gets recorded, but the
dirty bits it walks over are still cleared.

[1] https://lore.kernel.org/all/20261015003412.GA13920@nvidia.com/

Thanks,
  Andrea

Re: [PATCH 0/4] iommufd: Keep IOMMU_HWPT_GET_DIRTY_BITMAP within the user bitmap

[Jason Gunthorpe]

On Fri, 25 Sep 2026 18:11:36 +0200, Andrea Parri wrote:
> iommufd: Keep IOMMU_HWPT_GET_DIRTY_BITMAP within the user bitmap
> 
> IOMMU_HWPT_GET_DIRTY_BITMAP can set bits past the end of the bitmap the
> caller passed in. Two cases, both fixed here.
> 
> iova_bitmap_set() does not clip the range it records to the bitmap. When
> a query ends inside a dirty huge IOPTE, the generic page table code
> records the whole entry and the bits past the end of the user bitmap are
> set, up to the end of the last pinned page. The helper is shared with
> VFIO DMA logging, so the clip covers both users. Patch 1 fixes it and
> patch 2 tests it.
> 
> [...]

I picked up this patch:

[3/4] iommufd: Reject a zero-length dirty bitmap request
      https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/commit/?id=f6b3fd8f573e801066c21b2aec3d2e949e61e48e

And revised this one:

[1/4] iommufd/iova_bitmap: Clip recorded ranges to the bitmap
      https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/commit/?id=e21952d72b4740f08a272d694cca223b6b0099ca

As discussed.

This test doesn't work after the semantic change:

[2/4] iommufd/selftest: Test dirty bitmap recording past the query
      https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/commit/?id=00ee161c665f9e489bbd53c9185804d12e40d365

And I didn't think this one was so valuable:

[4/4] iommufd/selftest: Test a zero-length dirty bitmap request
      https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/commit/?id=9579527d416d00272a7e3b84330b6fcd9ea8cc31

So I left them out

Thanks,
Jason