[PATCH v3] iommu/iommufd: avoid selftest dirty bitmap size wrap

[PATCH v3] iommu/iommufd: avoid selftest dirty bitmap size wrap

[Samuel Moelius]

IOMMU_TEST_OP_DIRTY sizes its temporary dirty bitmap from
length / page_size.  Very large selftest ranges can make the
DIV_ROUND_UP() additions wrap before allocation, producing a
zero-length allocation while the later test_bit() loop still walks the
original number of bits.

The selftest helper does not need to support unbounded dirty bitmap
sizes.  Reject requests whose range cap calculation overflows, or whose
range exceeds SZ_16M pages, before converting the value to bitmap
allocation and copy sizes.

Assisted-by: Codex:gpt-5.5-cyber-preview
Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
---
Changes in v3:
  - Fail if page size is too large
Changes in v2:
  - Adjust how length is checked

 drivers/iommu/iommufd/selftest.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index af07c642a526..cf9a38e72b9a 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -12,6 +12,7 @@
 #include <linux/iommu.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/sizes.h>
 #include <linux/xarray.h>
 #include <uapi/linux/iommufd.h>
 #include <linux/generic_pt/iommu.h>
@@ -1686,6 +1687,8 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
 			      u32 flags)
 {
 	unsigned long i, max;
+	unsigned long max_pages = SZ_16M;
+	unsigned long max_length;
 	struct iommu_test_cmd *cmd = ucmd->cmd;
 	struct iommufd_hw_pagetable *hwpt;
 	struct mock_iommu_domain *mock;
@@ -1705,6 +1708,15 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
 		goto out_put;
 	}
 
+	if (check_mul_overflow(max_pages, page_size, &max_length)) {
+		rc = -EOVERFLOW;
+		goto out_put;
+	}
+	if (length > max_length) {
+		rc = -EOVERFLOW;
+		goto out_put;
+	}
+
 	max = length / page_size;
 	tmp = kvzalloc(DIV_ROUND_UP(max, BITS_PER_LONG) * sizeof(unsigned long),
 		       GFP_KERNEL_ACCOUNT);
-- 
2.43.0

Re: [PATCH v3] iommu/iommufd: avoid selftest dirty bitmap size wrap

[Jason Gunthorpe]

On Sun, Jun 28, 2026 at 03:23:32PM +0000, Samuel Moelius wrote:
> IOMMU_TEST_OP_DIRTY sizes its temporary dirty bitmap from
> length / page_size.  Very large selftest ranges can make the
> DIV_ROUND_UP() additions wrap before allocation, producing a
> zero-length allocation while the later test_bit() loop still walks the
> original number of bits.
> 
> The selftest helper does not need to support unbounded dirty bitmap
> sizes.  Reject requests whose range cap calculation overflows, or whose
> range exceeds SZ_16M pages, before converting the value to bitmap
> allocation and copy sizes.

I suggested to limit either the length or the kvzalloc to around 16M
as a practical limit

> @@ -1686,6 +1687,8 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
>  			      u32 flags)
>  {
>  	unsigned long i, max;
> +	unsigned long max_pages = SZ_16M;
> +	unsigned long max_length;
>  	struct iommu_test_cmd *cmd = ucmd->cmd;
>  	struct iommufd_hw_pagetable *hwpt;
>  	struct mock_iommu_domain *mock;
> @@ -1705,6 +1708,15 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
>  		goto out_put;
>  	}
>  
> +	if (check_mul_overflow(max_pages, page_size, &max_length)) {
> +		rc = -EOVERFLOW;
> +		goto out_put;
> +	}
> +	if (length > max_length) {
> +		rc = -EOVERFLOW;
> +		goto out_put;
> +	}

This is some huge number, so big that this breaks the logic on 32 bits.

It just needs this:

+       max = length / page_size;
+       if (max > SZ_16M * BITS_PER_BYTE)
+               return -EOVERFLOW;

At the top where it validates the arguments.

I revised this patch and merged it.

Jason

Re: [PATCH v3] iommu/iommufd: avoid selftest dirty bitmap size wrap

[Samuel Moelius]

On Mon, Jul 13, 2026 at 9:51 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Sun, Jun 28, 2026 at 03:23:32PM +0000, Samuel Moelius wrote:
> > IOMMU_TEST_OP_DIRTY sizes its temporary dirty bitmap from
> > length / page_size.  Very large selftest ranges can make the
> > DIV_ROUND_UP() additions wrap before allocation, producing a
> > zero-length allocation while the later test_bit() loop still walks the
> > original number of bits.
> >
> > The selftest helper does not need to support unbounded dirty bitmap
> > sizes.  Reject requests whose range cap calculation overflows, or whose
> > range exceeds SZ_16M pages, before converting the value to bitmap
> > allocation and copy sizes.
>
> I suggested to limit either the length or the kvzalloc to around 16M
> as a practical limit
>
> > @@ -1686,6 +1687,8 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
> >                             u32 flags)
> >  {
> >       unsigned long i, max;
> > +     unsigned long max_pages = SZ_16M;
> > +     unsigned long max_length;
> >       struct iommu_test_cmd *cmd = ucmd->cmd;
> >       struct iommufd_hw_pagetable *hwpt;
> >       struct mock_iommu_domain *mock;
> > @@ -1705,6 +1708,15 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
> >               goto out_put;
> >       }
> >
> > +     if (check_mul_overflow(max_pages, page_size, &max_length)) {
> > +             rc = -EOVERFLOW;
> > +             goto out_put;
> > +     }
> > +     if (length > max_length) {
> > +             rc = -EOVERFLOW;
> > +             goto out_put;
> > +     }
>
> This is some huge number, so big that this breaks the logic on 32 bits.
>
> It just needs this:
>
> +       max = length / page_size;
> +       if (max > SZ_16M * BITS_PER_BYTE)
> +               return -EOVERFLOW;
>
> At the top where it validates the arguments.
>
> I revised this patch and merged it.

Thank you, Jason.