Re: [PATCH] vfio: Validate that bitmap.pgsize is a power-of-2 in vfio_iommu_type1_unmap_dma

[Alex Williamson]

On Tue, 16 Jun 2026 19:17:33 +0800
lirongqing <lirongqing@xxxxxxxxx> wrote:

> From: Li RongQing <lirongqing@xxxxxxxxx>
> 
> In vfio_iommu_type1_unmap_dma(), the user-supplied bitmap.pgsize is
> passed directly to __ffs() without sufficient validation.
> 
> If userspace passes bitmap.pgsize == 0, it triggers undefined behavior
> in __ffs(), resulting in an undefined return value. Furthermore,
> passing a non-power-of-2 value (e.g., 3 or 5) results in an incorrect
> pgshift value. In either case, the invalid pgshift miscalculates the
> shifted unmap size (unmap.size >> pgshift), which distorts the
> subsequent verify_bitmap_size() validation logic and allows
> inconsistent user inputs to bypass proper sanitization.
> 
> Fix this by introducing an explicit is_power_of_2() check on
> bitmap.pgsize before processing. This strictly ensures the page size
> conforms to valid IOMMU page size semantics while preventing any
> downstream arithmetic anomalies caused by zero or non-power-of-2
> inputs.

For there to be an actual bug here, __ffs(0) would need to generate a
fault, not just return a garbage value.  If the value gets past
verify_bitmap_size() we call vfio_dma_do_unmap(), where we test
(bitmap->pgsize != pgsize), and pgsize is:

        pgshift = __ffs(iommu->pgsize_bitmap);
        pgsize = (size_t)1 << pgshift;

So, while I agree that we shouldn't be generating and passing around
garbage, the assertion that it allows user input to get past
sanitization isn't true.

The sashiko find is the same pattern, same downstream catch; __ffs()
returns a garbage value that's revalidated before use, never a fault,
so those sites are equally benign.

If you'd still like to pursue it, scope the commit log with the correct
severity, address the matching GET_BITMAP pgsize path, and repost.
Thanks,

Alex


> Fixes: 331e33d2960c8 ("vfio iommu: Update UNMAP_DMA ioctl to get
> dirty bitmap before unmap") Signed-off-by: Li RongQing
> <lirongqing@xxxxxxxxx> ---
>  drivers/vfio/vfio_iommu_type1.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c index c8151ba..b05fde8 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -38,6 +38,7 @@
>  #include <linux/workqueue.h>
>  #include <linux/mm_inline.h>
>  #include <linux/overflow.h>
> +#include <linux/log2.h>
>  #include "vfio.h"
>  
>  #define DRIVER_VERSION  "0.2"
> @@ -2949,6 +2950,9 @@ static int vfio_iommu_type1_unmap_dma(struct
> vfio_iommu *iommu, if (!access_ok((void __user *)bitmap.data,
> bitmap.size)) return -EINVAL;
>  
> +		if (unlikely(!is_power_of_2(bitmap.pgsize)))
> +			return -EINVAL;
> +
>  		pgshift = __ffs(bitmap.pgsize);
>  		ret = verify_bitmap_size(unmap.size >> pgshift,
>  					 bitmap.size);