public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test
@ 2026-03-03 19:46 Alex Mastro
  2026-03-04  0:47 ` Yao Yuan
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alex Mastro @ 2026-03-03 19:46 UTC (permalink / raw)
  To: Alex Williamson, David Matlack, Shuah Khan
  Cc: kvm, linux-kselftest, linux-kernel, Alex Mastro

Remove the __iommu_unmap() call on a region that was never mapped.
When __iommu_map() fails (expected for MMIO vaddrs in non-VFIO
modes), the region is not added to the dma_regions list, leaving its
list_head zero-initialized. If the unmap ioctl returns success,
__iommu_unmap() calls list_del_init() on this zeroed node and crashes.

This fixes the iommufd_compat_type1 and iommufd_compat_type1v2
test variants.

Fixes: 080723f4d4c3 ("vfio: selftests: Add vfio_dma_mapping_mmio_test")
Signed-off-by: Alex Mastro <amastro@fb.com>
---
The bug was missed because the test was originally run against a kernel
without commit afb47765f923 ("iommufd: Make vfio_compat's unmap succeed
if the range is already empty"). Without that fix, the unmap ioctl
returned -ENOENT, taking the early return before list_del_init().
---
 tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
index 957a89ce7b3a..d7f25ef77671 100644
--- a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
+++ b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
@@ -100,7 +100,6 @@ static void do_mmio_map_test(struct iommu *iommu,
 		iommu_unmap(iommu, &region);
 	} else {
 		VFIO_ASSERT_NE(__iommu_map(iommu, &region), 0);
-		VFIO_ASSERT_NE(__iommu_unmap(iommu, &region, NULL), 0);
 	}
 }
 

---
base-commit: 96ca4caf9066f5ebd35b561a521af588a8eb0215
change-id: 20260303-fix-mmio-test-d3bd688105f3

Best regards,
-- 
Alex Mastro <amastro@fb.com>


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

* Re: [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test
  2026-03-03 19:46 [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test Alex Mastro
@ 2026-03-04  0:47 ` Yao Yuan
  2026-03-04 19:10   ` Alex Mastro
  2026-03-04 22:30 ` David Matlack
  2026-03-20 21:18 ` Alex Williamson
  2 siblings, 1 reply; 5+ messages in thread
From: Yao Yuan @ 2026-03-04  0:47 UTC (permalink / raw)
  To: Alex Mastro
  Cc: Alex Williamson, David Matlack, Shuah Khan, kvm, linux-kselftest,
	linux-kernel

On Tue, Mar 03, 2026 at 11:46:24AM +0800, Alex Mastro wrote:
> Remove the __iommu_unmap() call on a region that was never mapped.
> When __iommu_map() fails (expected for MMIO vaddrs in non-VFIO
> modes), the region is not added to the dma_regions list, leaving its
> list_head zero-initialized. If the unmap ioctl returns success,
> __iommu_unmap() calls list_del_init() on this zeroed node and crashes.
>
> This fixes the iommufd_compat_type1 and iommufd_compat_type1v2
> test variants.
>
> Fixes: 080723f4d4c3 ("vfio: selftests: Add vfio_dma_mapping_mmio_test")
> Signed-off-by: Alex Mastro <amastro@fb.com>
> ---
> The bug was missed because the test was originally run against a kernel
> without commit afb47765f923 ("iommufd: Make vfio_compat's unmap succeed
> if the range is already empty"). Without that fix, the unmap ioctl
> returned -ENOENT, taking the early return before list_del_init().
> ---
>  tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> index 957a89ce7b3a..d7f25ef77671 100644
> --- a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> +++ b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> @@ -100,7 +100,6 @@ static void do_mmio_map_test(struct iommu *iommu,
>  		iommu_unmap(iommu, &region);
>  	} else {

Hi Alex,

>  		VFIO_ASSERT_NE(__iommu_map(iommu, &region), 0);
> -		VFIO_ASSERT_NE(__iommu_unmap(iommu, &region, NULL), 0);

This is the more simply way to work w/ or w/o commit
afb47765f923 ("iommufd: Make vfio_compat's unmap succeed if
the range is already empty"), may worth to add this point
into the commit message.

For the changes:

Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>

>  	}
>  }
>
>
> ---
> base-commit: 96ca4caf9066f5ebd35b561a521af588a8eb0215
> change-id: 20260303-fix-mmio-test-d3bd688105f3
>
> Best regards,
> --
> Alex Mastro <amastro@fb.com>
>

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

* Re: [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test
  2026-03-04  0:47 ` Yao Yuan
@ 2026-03-04 19:10   ` Alex Mastro
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Mastro @ 2026-03-04 19:10 UTC (permalink / raw)
  To: Yao Yuan
  Cc: Alex Williamson, David Matlack, Shuah Khan, kvm, linux-kselftest,
	linux-kernel

On Wed, Mar 04, 2026 at 08:47:15AM +0800, Yao Yuan wrote:
> On Tue, Mar 03, 2026 at 11:46:24AM +0800, Alex Mastro wrote:
> > Remove the __iommu_unmap() call on a region that was never mapped.
> > When __iommu_map() fails (expected for MMIO vaddrs in non-VFIO
> > modes), the region is not added to the dma_regions list, leaving its
> > list_head zero-initialized. If the unmap ioctl returns success,
> > __iommu_unmap() calls list_del_init() on this zeroed node and crashes.
> >
> > This fixes the iommufd_compat_type1 and iommufd_compat_type1v2
> > test variants.
> >
> > Fixes: 080723f4d4c3 ("vfio: selftests: Add vfio_dma_mapping_mmio_test")
> > Signed-off-by: Alex Mastro <amastro@fb.com>
> > ---
> > The bug was missed because the test was originally run against a kernel
> > without commit afb47765f923 ("iommufd: Make vfio_compat's unmap succeed
> > if the range is already empty"). Without that fix, the unmap ioctl
> > returned -ENOENT, taking the early return before list_del_init().
> > ---
> >  tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> > index 957a89ce7b3a..d7f25ef77671 100644
> > --- a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> > +++ b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> > @@ -100,7 +100,6 @@ static void do_mmio_map_test(struct iommu *iommu,
> >  		iommu_unmap(iommu, &region);
> >  	} else {
> 
> Hi Alex,
> 
> >  		VFIO_ASSERT_NE(__iommu_map(iommu, &region), 0);
> > -		VFIO_ASSERT_NE(__iommu_unmap(iommu, &region, NULL), 0);
> 
> This is the more simply way to work w/ or w/o commit
> afb47765f923 ("iommufd: Make vfio_compat's unmap succeed if
> the range is already empty"), may worth to add this point
> into the commit message.

True, but afb47765f923 was already in the vfio tree at the time the new test
was added in 080723f4d4c3. It was just (my own) negligence that I didn't run
the selftests against <current kernel>. I wrongly assumed that running the
selftests against an older kernel that I happened to have installed on my box
was good enough.

> 
> For the changes:
> 
> Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>

Thank you!

> 
> >  	}
> >  }
> >
> >
> > ---
> > base-commit: 96ca4caf9066f5ebd35b561a521af588a8eb0215
> > change-id: 20260303-fix-mmio-test-d3bd688105f3
> >
> > Best regards,
> > --
> > Alex Mastro <amastro@fb.com>
> >

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

* Re: [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test
  2026-03-03 19:46 [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test Alex Mastro
  2026-03-04  0:47 ` Yao Yuan
@ 2026-03-04 22:30 ` David Matlack
  2026-03-20 21:18 ` Alex Williamson
  2 siblings, 0 replies; 5+ messages in thread
From: David Matlack @ 2026-03-04 22:30 UTC (permalink / raw)
  To: Alex Mastro
  Cc: Alex Williamson, Shuah Khan, kvm, linux-kselftest, linux-kernel

On 2026-03-03 11:46 AM, Alex Mastro wrote:
> Remove the __iommu_unmap() call on a region that was never mapped.
> When __iommu_map() fails (expected for MMIO vaddrs in non-VFIO
> modes), the region is not added to the dma_regions list, leaving its
> list_head zero-initialized. If the unmap ioctl returns success,
> __iommu_unmap() calls list_del_init() on this zeroed node and crashes.
> 
> This fixes the iommufd_compat_type1 and iommufd_compat_type1v2
> test variants.
> 
> Fixes: 080723f4d4c3 ("vfio: selftests: Add vfio_dma_mapping_mmio_test")
> Signed-off-by: Alex Mastro <amastro@fb.com>
> ---
> The bug was missed because the test was originally run against a kernel
> without commit afb47765f923 ("iommufd: Make vfio_compat's unmap succeed
> if the range is already empty"). Without that fix, the unmap ioctl
> returned -ENOENT, taking the early return before list_del_init().

Thanks for the fix.

Looking back I remember testing your new test out without seeing a
failure. Turns out I had applied it on top of tag vfio-v6.19-rc1, which
did not have commit afb47765f923.

Reviewed-by: David Matlack <dmatlack@google.com>

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

* Re: [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test
  2026-03-03 19:46 [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test Alex Mastro
  2026-03-04  0:47 ` Yao Yuan
  2026-03-04 22:30 ` David Matlack
@ 2026-03-20 21:18 ` Alex Williamson
  2 siblings, 0 replies; 5+ messages in thread
From: Alex Williamson @ 2026-03-20 21:18 UTC (permalink / raw)
  To: Alex Mastro
  Cc: David Matlack, Shuah Khan, kvm, linux-kselftest, linux-kernel,
	alex

On Tue, 3 Mar 2026 11:46:24 -0800
Alex Mastro <amastro@fb.com> wrote:

> Remove the __iommu_unmap() call on a region that was never mapped.
> When __iommu_map() fails (expected for MMIO vaddrs in non-VFIO
> modes), the region is not added to the dma_regions list, leaving its
> list_head zero-initialized. If the unmap ioctl returns success,
> __iommu_unmap() calls list_del_init() on this zeroed node and crashes.
> 
> This fixes the iommufd_compat_type1 and iommufd_compat_type1v2
> test variants.
> 
> Fixes: 080723f4d4c3 ("vfio: selftests: Add vfio_dma_mapping_mmio_test")
> Signed-off-by: Alex Mastro <amastro@fb.com>
> ---
> The bug was missed because the test was originally run against a kernel
> without commit afb47765f923 ("iommufd: Make vfio_compat's unmap succeed
> if the range is already empty"). Without that fix, the unmap ioctl
> returned -ENOENT, taking the early return before list_del_init().
> ---
>  tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> index 957a89ce7b3a..d7f25ef77671 100644
> --- a/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> +++ b/tools/testing/selftests/vfio/vfio_dma_mapping_mmio_test.c
> @@ -100,7 +100,6 @@ static void do_mmio_map_test(struct iommu *iommu,
>  		iommu_unmap(iommu, &region);
>  	} else {
>  		VFIO_ASSERT_NE(__iommu_map(iommu, &region), 0);
> -		VFIO_ASSERT_NE(__iommu_unmap(iommu, &region, NULL), 0);
>  	}
>  }
>  
> 
> ---
> base-commit: 96ca4caf9066f5ebd35b561a521af588a8eb0215
> change-id: 20260303-fix-mmio-test-d3bd688105f3

Applied to vfio next branch for v7.1.  Thanks,

Alex

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

end of thread, other threads:[~2026-03-20 21:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03 19:46 [PATCH] vfio: selftests: fix crash in vfio_dma_mapping_mmio_test Alex Mastro
2026-03-04  0:47 ` Yao Yuan
2026-03-04 19:10   ` Alex Mastro
2026-03-04 22:30 ` David Matlack
2026-03-20 21:18 ` Alex Williamson

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