mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] vfio: Validate that bitmap.pgsize is a power-of-2 in vfio_iommu_type1_unmap_dma
@ 2026-06-16 11:17 lirongqing
  2026-06-16 23:14 ` Alex Williamson
  0 siblings, 1 reply; 2+ messages in thread
From: lirongqing @ 2026-06-16 11:17 UTC (permalink / raw)
  To: Alex Williamson, kvm, linux-kernel; +Cc: Li RongQing

From: Li RongQing <lirongqing@baidu.com>

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.

Fixes: 331e33d2960c8 ("vfio iommu: Update UNMAP_DMA ioctl to get dirty bitmap before unmap")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 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);
-- 
2.9.4


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] vfio: Validate that bitmap.pgsize is a power-of-2 in vfio_iommu_type1_unmap_dma
  2026-06-16 11:17 [PATCH] vfio: Validate that bitmap.pgsize is a power-of-2 in vfio_iommu_type1_unmap_dma lirongqing
@ 2026-06-16 23:14 ` Alex Williamson
  0 siblings, 0 replies; 2+ messages in thread
From: Alex Williamson @ 2026-06-16 23:14 UTC (permalink / raw)
  To: lirongqing; +Cc: kvm, linux-kernel, alex

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

> From: Li RongQing <lirongqing@baidu.com>
> 
> 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@baidu.com> ---
>  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);


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-16 23:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-16 11:17 [PATCH] vfio: Validate that bitmap.pgsize is a power-of-2 in vfio_iommu_type1_unmap_dma lirongqing
2026-06-16 23:14 ` Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome