[PATCH] vfio/type1: validate dirty bitmap page size before use

[PATCH] vfio/type1: validate dirty bitmap page size before use

[Yousef Alhouseen]

The dirty bitmap ioctl paths derive a shift with __ffs() from the
userspace supplied bitmap page size before comparing it with the IOMMU
page size. A zero page size has undefined behavior, and a non-power-of-2
value makes the bitmap size check use a different granularity than the
page size later accepted by the ioctl.

Reject invalid bitmap page sizes before using them in shift arithmetic.

Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/vfio/vfio_iommu_type1.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index c8151ba54..9499381d0 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -26,6 +26,7 @@
 #include <linux/fs.h>
 #include <linux/highmem.h>
 #include <linux/iommu.h>
+#include <linux/log2.h>
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/kthread.h>
@@ -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 (!is_power_of_2(bitmap.pgsize))
+			return -EINVAL;
+
 		pgshift = __ffs(bitmap.pgsize);
 		ret = verify_bitmap_size(unmap.size >> pgshift,
 					 bitmap.size);
@@ -3039,6 +3043,9 @@ static int vfio_iommu_type1_dirty_pages(struct vfio_iommu *iommu,
 			       range.bitmap.size))
 			return -EINVAL;
 
+		if (!is_power_of_2(range.bitmap.pgsize))
+			return -EINVAL;
+
 		pgshift = __ffs(range.bitmap.pgsize);
 		ret = verify_bitmap_size(size >> pgshift,
 					 range.bitmap.size);
-- 
2.54.0

Re: [PATCH] vfio/type1: validate dirty bitmap page size before use

[Alex Williamson]

On Wed, 24 Jun 2026 21:12:04 +0200
Yousef Alhouseen <alhouseenyousef@gmail.com> wrote:

> The dirty bitmap ioctl paths derive a shift with __ffs() from the
> userspace supplied bitmap page size before comparing it with the IOMMU
> page size. A zero page size has undefined behavior, and a non-power-of-2
> value makes the bitmap size check use a different granularity than the
> page size later accepted by the ioctl.
> 
> Reject invalid bitmap page sizes before using them in shift arithmetic.

Thanks for the patch, this is already being worked:

[v1]https://lore.kernel.org/all/20260616111733.1810-1-lirongqing@baidu.com/
[v2]https://lore.kernel.org/all/20260617113251.2535-1-lirongqing@baidu.com/

Thanks,
Alex

> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index c8151ba54..9499381d0 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -26,6 +26,7 @@
>  #include <linux/fs.h>
>  #include <linux/highmem.h>
>  #include <linux/iommu.h>
> +#include <linux/log2.h>
>  #include <linux/module.h>
>  #include <linux/mm.h>
>  #include <linux/kthread.h>
> @@ -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 (!is_power_of_2(bitmap.pgsize))
> +			return -EINVAL;
> +
>  		pgshift = __ffs(bitmap.pgsize);
>  		ret = verify_bitmap_size(unmap.size >> pgshift,
>  					 bitmap.size);
> @@ -3039,6 +3043,9 @@ static int vfio_iommu_type1_dirty_pages(struct vfio_iommu *iommu,
>  			       range.bitmap.size))
>  			return -EINVAL;
>  
> +		if (!is_power_of_2(range.bitmap.pgsize))
> +			return -EINVAL;
> +
>  		pgshift = __ffs(range.bitmap.pgsize);
>  		ret = verify_bitmap_size(size >> pgshift,
>  					 range.bitmap.size);

Re: [PATCH] vfio/type1: validate dirty bitmap page size before use

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling `__ffs()` and `__fls()` with an argument of 0 results in undefined behavior when userspace passes `dirty.flags = 0`.
--

commit 38ca5c3a7cef062b9bd643fc4332459f28940553
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>

vfio/type1: validate dirty bitmap page size before use

The dirty bitmap ioctl paths derive a shift with __ffs() from the userspace
supplied bitmap page size before comparing it with the IOMMU page size. A zero
page size has undefined behavior, and a non-power-of-2 value makes the bitmap
size check use a different granularity than the page size later accepted by
the ioctl.

> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index c8151ba54de33..9499381d06a20 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c

[ ... ]

> @@ -3039,6 +3043,9 @@ static int vfio_iommu_type1_dirty_pages(struct vfio_iommu *iommu,

[Severity: High]
This isn't a bug introduced by this patch, but does calling __ffs() and
__fls() with an argument of 0 result in undefined behavior when userspace
passes dirty.flags set to 0?

In vfio_iommu_type1_dirty_pages():

	if (dirty.argsz < minsz || dirty.flags & ~mask)
		return -EINVAL;

	/* only one flag should be set at a time */
	if (__ffs(dirty.flags) != __fls(dirty.flags))
		return -EINVAL;

If dirty.flags is 0, the mask check evaluates to 0 and passes, allowing
__ffs(0) and __fls(0) to execute. Since these macros map to __builtin_ctzl
and __builtin_clzl, this results in undefined behavior.

Could this be triggered by userspace to cause a denial of service?

>  			       range.bitmap.size))
>  			return -EINVAL;
>  
> +		if (!is_power_of_2(range.bitmap.pgsize))
> +			return -EINVAL;
> +
>  		pgshift = __ffs(range.bitmap.pgsize);
>  		ret = verify_bitmap_size(size >> pgshift,
>  					 range.bitmap.size);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624191204.3774-1-alhouseenyousef@gmail.com?part=1