All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix undetected overflow when allocating IOVA
@ 2025-07-17 19:15 Jason Gunthorpe
  2025-07-17 19:15 ` [PATCH 1/2] iommufd: Prevent ALIGN() overflow Jason Gunthorpe
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2025-07-17 19:15 UTC (permalink / raw)
  To: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon
  Cc: Lixiao Yang, Matthew Rosato, Nicolin Chen, patches, stable,
	syzbot+c2f65e2801743ca64e08, Yi Liu

Syzkaller found this, the ALIGN() call can overflow and corrupt the
allocation process. Fix the bug and add some test coverage.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Jason Gunthorpe (2):
  iommufd: Prevent ALIGN() overflow
  iommufd/selftest: Test reserved regions near ULONG_MAX

 drivers/iommu/iommufd/io_pagetable.c    | 41 +++++++++++++++----------
 tools/testing/selftests/iommu/iommufd.c | 18 +++++++++++
 2 files changed, 43 insertions(+), 16 deletions(-)


base-commit: 601b1d0d9395c711383452bd0d47037afbbb4bcf
-- 
2.43.0


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

* [PATCH 1/2] iommufd: Prevent ALIGN() overflow
  2025-07-17 19:15 [PATCH 0/2] Fix undetected overflow when allocating IOVA Jason Gunthorpe
@ 2025-07-17 19:15 ` Jason Gunthorpe
  2025-07-18  8:16   ` Nicolin Chen
  2025-07-18 13:03   ` Yi Liu
  2025-07-17 19:15 ` [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX Jason Gunthorpe
  2025-07-18 18:10 ` [PATCH 0/2] Fix undetected overflow when allocating IOVA Nicolin Chen
  2 siblings, 2 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2025-07-17 19:15 UTC (permalink / raw)
  To: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon
  Cc: Lixiao Yang, Matthew Rosato, Nicolin Chen, patches, stable,
	syzbot+c2f65e2801743ca64e08, Yi Liu

When allocating IOVA the candidate range gets aligned to the target
alignment. If the range is close to ULONG_MAX then the ALIGN() can
wrap resulting in a corrupted iova.

Open code the ALIGN() using get_add_overflow() to prevent this.
This simplifies the checks as we don't need to check for length earlier
either.

Consolidate the two copies of this code under a single helper.

This bug would allow userspace to create a mapping that overlaps with some
other mapping or a reserved range.

Cc: stable@vger.kernel.org
Fixes: 51fe6141f0f6 ("iommufd: Data structure to provide IOVA to PFN mapping")
Reported-by: syzbot+c2f65e2801743ca64e08@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/685af644.a00a0220.2e5631.0094.GAE@google.com
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/iommufd/io_pagetable.c | 41 +++++++++++++++++-----------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/iommufd/io_pagetable.c b/drivers/iommu/iommufd/io_pagetable.c
index abf4aadca96c0b..c0360c450880b8 100644
--- a/drivers/iommu/iommufd/io_pagetable.c
+++ b/drivers/iommu/iommufd/io_pagetable.c
@@ -70,20 +70,34 @@ struct iopt_area *iopt_area_contig_next(struct iopt_area_contig_iter *iter)
 	return iter->area;
 }
 
+static bool __alloc_iova_check_range(unsigned long *start, unsigned long last,
+				     unsigned long length,
+				     unsigned long iova_alignment,
+				     unsigned long page_offset)
+{
+	unsigned long aligned_start;
+
+	/* ALIGN_UP() */
+	if (check_add_overflow(*start, iova_alignment - 1, &aligned_start))
+		return false;
+	aligned_start &= ~(iova_alignment - 1);
+	aligned_start |= page_offset;
+
+	if (aligned_start >= last || last - aligned_start < length - 1)
+		return false;
+	*start = aligned_start;
+	return true;
+}
+
 static bool __alloc_iova_check_hole(struct interval_tree_double_span_iter *span,
 				    unsigned long length,
 				    unsigned long iova_alignment,
 				    unsigned long page_offset)
 {
-	if (span->is_used || span->last_hole - span->start_hole < length - 1)
+	if (span->is_used)
 		return false;
-
-	span->start_hole = ALIGN(span->start_hole, iova_alignment) |
-			   page_offset;
-	if (span->start_hole > span->last_hole ||
-	    span->last_hole - span->start_hole < length - 1)
-		return false;
-	return true;
+	return __alloc_iova_check_range(&span->start_hole, span->last_hole,
+					length, iova_alignment, page_offset);
 }
 
 static bool __alloc_iova_check_used(struct interval_tree_span_iter *span,
@@ -91,15 +105,10 @@ static bool __alloc_iova_check_used(struct interval_tree_span_iter *span,
 				    unsigned long iova_alignment,
 				    unsigned long page_offset)
 {
-	if (span->is_hole || span->last_used - span->start_used < length - 1)
+	if (span->is_hole)
 		return false;
-
-	span->start_used = ALIGN(span->start_used, iova_alignment) |
-			   page_offset;
-	if (span->start_used > span->last_used ||
-	    span->last_used - span->start_used < length - 1)
-		return false;
-	return true;
+	return __alloc_iova_check_range(&span->start_used, span->last_used,
+					length, iova_alignment, page_offset);
 }
 
 /*
-- 
2.43.0


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

* [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-17 19:15 [PATCH 0/2] Fix undetected overflow when allocating IOVA Jason Gunthorpe
  2025-07-17 19:15 ` [PATCH 1/2] iommufd: Prevent ALIGN() overflow Jason Gunthorpe
@ 2025-07-17 19:15 ` Jason Gunthorpe
  2025-07-17 19:15   ` kernel test robot
                     ` (2 more replies)
  2025-07-18 18:10 ` [PATCH 0/2] Fix undetected overflow when allocating IOVA Nicolin Chen
  2 siblings, 3 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2025-07-17 19:15 UTC (permalink / raw)
  To: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon
  Cc: Lixiao Yang, Matthew Rosato, Nicolin Chen, patches, stable,
	syzbot+c2f65e2801743ca64e08, Yi Liu

This has triggered an overflow inside the ioas iova auto allocation logic,
test it directly. Use the same stimulus syzkaller found.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 tools/testing/selftests/iommu/iommufd.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index d59d48022a24af..d9df92e27264b1 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -968,6 +968,24 @@ TEST_F(iommufd_ioas, area_auto_iova)
 		test_ioctl_ioas_unmap(iovas[i], PAGE_SIZE * (i + 1));
 }
 
+/*  https://lore.kernel.org/r/685af644.a00a0220.2e5631.0094.GAE@google.com */
+TEST_F(iommufd_ioas, reserved_overflow)
+{
+	struct iommu_test_cmd test_cmd = {
+		.size = sizeof(test_cmd),
+		.op = IOMMU_TEST_OP_ADD_RESERVED,
+		.id = self->ioas_id,
+		.add_reserved = { .start = 6,
+				  .length = 0xffffffffffff8001 },
+	};
+	__u64 iova;
+
+	ASSERT_EQ(0,
+		  ioctl(self->fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_ADD_RESERVED),
+			&test_cmd));
+	test_err_ioctl_ioas_map(ENOSPC, buffer, 0x5000, &iova);
+}
+
 TEST_F(iommufd_ioas, area_allowed)
 {
 	struct iommu_test_cmd test_cmd = {
-- 
2.43.0


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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-17 19:15 ` [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX Jason Gunthorpe
@ 2025-07-17 19:15   ` kernel test robot
  2025-07-18  2:45   ` Nicolin Chen
  2025-07-18 13:03   ` Yi Liu
  2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2025-07-17 19:15 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: stable, oe-kbuild-all

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree.
Subject: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
Link: https://lore.kernel.org/stable/2-v1-7b4a16fc390b%2B10f4-iommufd_alloc_overflow_jgg%40nvidia.com

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki




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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-17 19:15 ` [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX Jason Gunthorpe
  2025-07-17 19:15   ` kernel test robot
@ 2025-07-18  2:45   ` Nicolin Chen
  2025-07-18  8:13     ` Nicolin Chen
  2025-07-18 13:03   ` Yi Liu
  2 siblings, 1 reply; 15+ messages in thread
From: Nicolin Chen @ 2025-07-18  2:45 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Thu, Jul 17, 2025 at 04:15:09PM -0300, Jason Gunthorpe wrote:

> +TEST_F(iommufd_ioas, reserved_overflow)
> +{
> +	struct iommu_test_cmd test_cmd = {
> +		.size = sizeof(test_cmd),
> +		.op = IOMMU_TEST_OP_ADD_RESERVED,
> +		.id = self->ioas_id,
> +		.add_reserved = { .start = 6,
> +				  .length = 0xffffffffffff8001 },
> +	};
> +	__u64 iova;
> +
> +	ASSERT_EQ(0,
> +		  ioctl(self->fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_ADD_RESERVED),
> +			&test_cmd));
> +	test_err_ioctl_ioas_map(ENOSPC, buffer, 0x5000, &iova);

When:
PAGE_SIZE=SZ_64K = 0x10000
MOCK_PAGE_SIZE = PAGE_SIZE / 2 = 0x8000

This likely fails the alignment test, returning -EINVAL instead:

# iommufd.c:988:reserved_overflow:Expected 28 (28) == errno (22)
# reserved_overflow: Test failed
#          FAIL  iommufd_ioas.mock_domain_limit.reserved_overflow

So, I think we'd have to pick a number aligned to MOCK_PAGE_SIZE?
e.g. changing to 0x18000 for example can pass.

Thanks
Nicolin

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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-18  2:45   ` Nicolin Chen
@ 2025-07-18  8:13     ` Nicolin Chen
  2025-07-18 16:21       ` Jason Gunthorpe
  2025-07-18 18:23       ` Robin Murphy
  0 siblings, 2 replies; 15+ messages in thread
From: Nicolin Chen @ 2025-07-18  8:13 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Thu, Jul 17, 2025 at 07:45:51PM -0700, Nicolin Chen wrote:
> On Thu, Jul 17, 2025 at 04:15:09PM -0300, Jason Gunthorpe wrote:
> 
> > +TEST_F(iommufd_ioas, reserved_overflow)
> > +{
> > +	struct iommu_test_cmd test_cmd = {
> > +		.size = sizeof(test_cmd),
> > +		.op = IOMMU_TEST_OP_ADD_RESERVED,
> > +		.id = self->ioas_id,
> > +		.add_reserved = { .start = 6,
> > +				  .length = 0xffffffffffff8001 },
> > +	};
> > +	__u64 iova;
> > +
> > +	ASSERT_EQ(0,
> > +		  ioctl(self->fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_ADD_RESERVED),
> > +			&test_cmd));
> > +	test_err_ioctl_ioas_map(ENOSPC, buffer, 0x5000, &iova);
> 
> When:
> PAGE_SIZE=SZ_64K = 0x10000
> MOCK_PAGE_SIZE = PAGE_SIZE / 2 = 0x8000
> 
> This likely fails the alignment test, returning -EINVAL instead:
> 
> # iommufd.c:988:reserved_overflow:Expected 28 (28) == errno (22)
> # reserved_overflow: Test failed
> #          FAIL  iommufd_ioas.mock_domain_limit.reserved_overflow
> 
> So, I think we'd have to pick a number aligned to MOCK_PAGE_SIZE?
> e.g. changing to 0x18000 for example can pass.

I realized that we can't change the number as it won't reproduce
on PAGE_SIZE=4K. So, perhaps it should just SKIP other page sizes
than 4K.

Thanks
Nicolin

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

* Re: [PATCH 1/2] iommufd: Prevent ALIGN() overflow
  2025-07-17 19:15 ` [PATCH 1/2] iommufd: Prevent ALIGN() overflow Jason Gunthorpe
@ 2025-07-18  8:16   ` Nicolin Chen
  2025-07-18 13:03   ` Yi Liu
  1 sibling, 0 replies; 15+ messages in thread
From: Nicolin Chen @ 2025-07-18  8:16 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Thu, Jul 17, 2025 at 04:15:08PM -0300, Jason Gunthorpe wrote:
> When allocating IOVA the candidate range gets aligned to the target
> alignment. If the range is close to ULONG_MAX then the ALIGN() can
> wrap resulting in a corrupted iova.
> 
> Open code the ALIGN() using get_add_overflow() to prevent this.
> This simplifies the checks as we don't need to check for length earlier
> either.
> 
> Consolidate the two copies of this code under a single helper.
> 
> This bug would allow userspace to create a mapping that overlaps with some
> other mapping or a reserved range.
> 
> Cc: stable@vger.kernel.org
> Fixes: 51fe6141f0f6 ("iommufd: Data structure to provide IOVA to PFN mapping")
> Reported-by: syzbot+c2f65e2801743ca64e08@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/r/685af644.a00a0220.2e5631.0094.GAE@google.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
 
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

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

* Re: [PATCH 1/2] iommufd: Prevent ALIGN() overflow
  2025-07-17 19:15 ` [PATCH 1/2] iommufd: Prevent ALIGN() overflow Jason Gunthorpe
  2025-07-18  8:16   ` Nicolin Chen
@ 2025-07-18 13:03   ` Yi Liu
  1 sibling, 0 replies; 15+ messages in thread
From: Yi Liu @ 2025-07-18 13:03 UTC (permalink / raw)
  To: Jason Gunthorpe, iommu, Joerg Roedel, Kevin Tian, linux-kselftest,
	Robin Murphy, Shuah Khan, Will Deacon
  Cc: Lixiao Yang, Matthew Rosato, Nicolin Chen, patches, stable,
	syzbot+c2f65e2801743ca64e08

On 2025/7/18 03:15, Jason Gunthorpe wrote:
> When allocating IOVA the candidate range gets aligned to the target
> alignment. If the range is close to ULONG_MAX then the ALIGN() can
> wrap resulting in a corrupted iova.
> 
> Open code the ALIGN() using get_add_overflow() to prevent this.
> This simplifies the checks as we don't need to check for length earlier
> either.
> 
> Consolidate the two copies of this code under a single helper.
> 
> This bug would allow userspace to create a mapping that overlaps with some
> other mapping or a reserved range.
> 
> Cc: stable@vger.kernel.org
> Fixes: 51fe6141f0f6 ("iommufd: Data structure to provide IOVA to PFN mapping")
> Reported-by: syzbot+c2f65e2801743ca64e08@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/r/685af644.a00a0220.2e5631.0094.GAE@google.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   drivers/iommu/iommufd/io_pagetable.c | 41 +++++++++++++++++-----------
>   1 file changed, 25 insertions(+), 16 deletions(-)

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

> diff --git a/drivers/iommu/iommufd/io_pagetable.c b/drivers/iommu/iommufd/io_pagetable.c
> index abf4aadca96c0b..c0360c450880b8 100644
> --- a/drivers/iommu/iommufd/io_pagetable.c
> +++ b/drivers/iommu/iommufd/io_pagetable.c
> @@ -70,20 +70,34 @@ struct iopt_area *iopt_area_contig_next(struct iopt_area_contig_iter *iter)
>   	return iter->area;
>   }
>   
> +static bool __alloc_iova_check_range(unsigned long *start, unsigned long last,
> +				     unsigned long length,
> +				     unsigned long iova_alignment,
> +				     unsigned long page_offset)
> +{
> +	unsigned long aligned_start;
> +
> +	/* ALIGN_UP() */
> +	if (check_add_overflow(*start, iova_alignment - 1, &aligned_start))
> +		return false;
> +	aligned_start &= ~(iova_alignment - 1);
> +	aligned_start |= page_offset;
> +
> +	if (aligned_start >= last || last - aligned_start < length - 1)
> +		return false;
> +	*start = aligned_start;
> +	return true;
> +}
> +
>   static bool __alloc_iova_check_hole(struct interval_tree_double_span_iter *span,
>   				    unsigned long length,
>   				    unsigned long iova_alignment,
>   				    unsigned long page_offset)
>   {
> -	if (span->is_used || span->last_hole - span->start_hole < length - 1)
> +	if (span->is_used)
>   		return false;
> -
> -	span->start_hole = ALIGN(span->start_hole, iova_alignment) |
> -			   page_offset;
> -	if (span->start_hole > span->last_hole ||
> -	    span->last_hole - span->start_hole < length - 1)
> -		return false;
> -	return true;
> +	return __alloc_iova_check_range(&span->start_hole, span->last_hole,
> +					length, iova_alignment, page_offset);
>   }
>   
>   static bool __alloc_iova_check_used(struct interval_tree_span_iter *span,
> @@ -91,15 +105,10 @@ static bool __alloc_iova_check_used(struct interval_tree_span_iter *span,
>   				    unsigned long iova_alignment,
>   				    unsigned long page_offset)
>   {
> -	if (span->is_hole || span->last_used - span->start_used < length - 1)
> +	if (span->is_hole)
>   		return false;
> -
> -	span->start_used = ALIGN(span->start_used, iova_alignment) |
> -			   page_offset;
> -	if (span->start_used > span->last_used ||
> -	    span->last_used - span->start_used < length - 1)
> -		return false;
> -	return true;
> +	return __alloc_iova_check_range(&span->start_used, span->last_used,
> +					length, iova_alignment, page_offset);
>   }
>   
>   /*

-- 
Regards,
Yi Liu

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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-17 19:15 ` [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX Jason Gunthorpe
  2025-07-17 19:15   ` kernel test robot
  2025-07-18  2:45   ` Nicolin Chen
@ 2025-07-18 13:03   ` Yi Liu
  2 siblings, 0 replies; 15+ messages in thread
From: Yi Liu @ 2025-07-18 13:03 UTC (permalink / raw)
  To: Jason Gunthorpe, iommu, Joerg Roedel, Kevin Tian, linux-kselftest,
	Robin Murphy, Shuah Khan, Will Deacon
  Cc: Lixiao Yang, Matthew Rosato, Nicolin Chen, patches, stable,
	syzbot+c2f65e2801743ca64e08

On 2025/7/18 03:15, Jason Gunthorpe wrote:
> This has triggered an overflow inside the ioas iova auto allocation logic,
> test it directly. Use the same stimulus syzkaller found.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   tools/testing/selftests/iommu/iommufd.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)

run it on intel x86. this test case failed without patch 01, and passed
with patch 01.

Tested-by: Yi Liu <yi.l.liu@intel.com>

> diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
> index d59d48022a24af..d9df92e27264b1 100644
> --- a/tools/testing/selftests/iommu/iommufd.c
> +++ b/tools/testing/selftests/iommu/iommufd.c
> @@ -968,6 +968,24 @@ TEST_F(iommufd_ioas, area_auto_iova)
>   		test_ioctl_ioas_unmap(iovas[i], PAGE_SIZE * (i + 1));
>   }
>   
> +/*  https://lore.kernel.org/r/685af644.a00a0220.2e5631.0094.GAE@google.com */
> +TEST_F(iommufd_ioas, reserved_overflow)
> +{
> +	struct iommu_test_cmd test_cmd = {
> +		.size = sizeof(test_cmd),
> +		.op = IOMMU_TEST_OP_ADD_RESERVED,
> +		.id = self->ioas_id,
> +		.add_reserved = { .start = 6,
> +				  .length = 0xffffffffffff8001 },
> +	};
> +	__u64 iova;
> +
> +	ASSERT_EQ(0,
> +		  ioctl(self->fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_ADD_RESERVED),
> +			&test_cmd));
> +	test_err_ioctl_ioas_map(ENOSPC, buffer, 0x5000, &iova);
> +}
> +
>   TEST_F(iommufd_ioas, area_allowed)
>   {
>   	struct iommu_test_cmd test_cmd = {

-- 
Regards,
Yi Liu

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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-18  8:13     ` Nicolin Chen
@ 2025-07-18 16:21       ` Jason Gunthorpe
  2025-07-18 18:23       ` Robin Murphy
  1 sibling, 0 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2025-07-18 16:21 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Fri, Jul 18, 2025 at 01:13:46AM -0700, Nicolin Chen wrote:

> I realized that we can't change the number as it won't reproduce
> on PAGE_SIZE=4K. So, perhaps it should just SKIP other page sizes
> than 4K.

Ok, I used this:

+       if (PAGE_SIZE != 4096)
+               SKIP(return, "Test requires 4k PAGE_SIZE");
+

Thanks,
Jason

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

* Re: [PATCH 0/2] Fix undetected overflow when allocating IOVA
  2025-07-17 19:15 [PATCH 0/2] Fix undetected overflow when allocating IOVA Jason Gunthorpe
  2025-07-17 19:15 ` [PATCH 1/2] iommufd: Prevent ALIGN() overflow Jason Gunthorpe
  2025-07-17 19:15 ` [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX Jason Gunthorpe
@ 2025-07-18 18:10 ` Nicolin Chen
  2 siblings, 0 replies; 15+ messages in thread
From: Nicolin Chen @ 2025-07-18 18:10 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Thu, Jul 17, 2025 at 04:15:07PM -0300, Jason Gunthorpe wrote:
> Syzkaller found this, the ALIGN() call can overflow and corrupt the
> allocation process. Fix the bug and add some test coverage.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> Jason Gunthorpe (2):
>   iommufd: Prevent ALIGN() overflow
>   iommufd/selftest: Test reserved regions near ULONG_MAX

With the SKIP in PATCH-2:

Tested-by: Nicolin Chen <nicolinc@nvidia.com>

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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-18  8:13     ` Nicolin Chen
  2025-07-18 16:21       ` Jason Gunthorpe
@ 2025-07-18 18:23       ` Robin Murphy
  2025-07-18 18:50         ` Nicolin Chen
  2025-07-18 19:56         ` Jason Gunthorpe
  1 sibling, 2 replies; 15+ messages in thread
From: Robin Murphy @ 2025-07-18 18:23 UTC (permalink / raw)
  To: Nicolin Chen, Jason Gunthorpe
  Cc: iommu, Joerg Roedel, Kevin Tian, linux-kselftest, Shuah Khan,
	Will Deacon, Lixiao Yang, Matthew Rosato, patches, stable,
	syzbot+c2f65e2801743ca64e08, Yi Liu

On 2025-07-18 9:13 am, Nicolin Chen wrote:
> On Thu, Jul 17, 2025 at 07:45:51PM -0700, Nicolin Chen wrote:
>> On Thu, Jul 17, 2025 at 04:15:09PM -0300, Jason Gunthorpe wrote:
>>
>>> +TEST_F(iommufd_ioas, reserved_overflow)
>>> +{
>>> +	struct iommu_test_cmd test_cmd = {
>>> +		.size = sizeof(test_cmd),
>>> +		.op = IOMMU_TEST_OP_ADD_RESERVED,
>>> +		.id = self->ioas_id,
>>> +		.add_reserved = { .start = 6,
>>> +				  .length = 0xffffffffffff8001 },
>>> +	};
>>> +	__u64 iova;
>>> +
>>> +	ASSERT_EQ(0,
>>> +		  ioctl(self->fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_ADD_RESERVED),
>>> +			&test_cmd));
>>> +	test_err_ioctl_ioas_map(ENOSPC, buffer, 0x5000, &iova);
>>
>> When:
>> PAGE_SIZE=SZ_64K = 0x10000
>> MOCK_PAGE_SIZE = PAGE_SIZE / 2 = 0x8000
>>
>> This likely fails the alignment test, returning -EINVAL instead:
>>
>> # iommufd.c:988:reserved_overflow:Expected 28 (28) == errno (22)
>> # reserved_overflow: Test failed
>> #          FAIL  iommufd_ioas.mock_domain_limit.reserved_overflow
>>
>> So, I think we'd have to pick a number aligned to MOCK_PAGE_SIZE?
>> e.g. changing to 0x18000 for example can pass.
> 
> I realized that we can't change the number as it won't reproduce
> on PAGE_SIZE=4K. So, perhaps it should just SKIP other page sizes
> than 4K.

Shouldn't it work to parametrise both numbers accordingly, e.g. (if my 
Friday-evening maths holds up) reserve "1UL - MOCK_PAGE_SIZE * 16" then 
map "MOCK_PAGE_SIZE * 10"?

Robin.

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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-18 18:23       ` Robin Murphy
@ 2025-07-18 18:50         ` Nicolin Chen
  2025-07-18 20:16           ` Jason Gunthorpe
  2025-07-18 19:56         ` Jason Gunthorpe
  1 sibling, 1 reply; 15+ messages in thread
From: Nicolin Chen @ 2025-07-18 18:50 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Jason Gunthorpe, iommu, Joerg Roedel, Kevin Tian, linux-kselftest,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Fri, Jul 18, 2025 at 07:23:25PM +0100, Robin Murphy wrote:
> On 2025-07-18 9:13 am, Nicolin Chen wrote:
> > On Thu, Jul 17, 2025 at 07:45:51PM -0700, Nicolin Chen wrote:
> > > On Thu, Jul 17, 2025 at 04:15:09PM -0300, Jason Gunthorpe wrote:
> > > 
> > > > +TEST_F(iommufd_ioas, reserved_overflow)
> > > > +{
> > > > +	struct iommu_test_cmd test_cmd = {
> > > > +		.size = sizeof(test_cmd),
> > > > +		.op = IOMMU_TEST_OP_ADD_RESERVED,
> > > > +		.id = self->ioas_id,
> > > > +		.add_reserved = { .start = 6,
> > > > +				  .length = 0xffffffffffff8001 },
> > > > +	};
> > > > +	__u64 iova;
> > > > +
> > > > +	ASSERT_EQ(0,
> > > > +		  ioctl(self->fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_ADD_RESERVED),
> > > > +			&test_cmd));
> > > > +	test_err_ioctl_ioas_map(ENOSPC, buffer, 0x5000, &iova);
> > > 
> > > When:
> > > PAGE_SIZE=SZ_64K = 0x10000
> > > MOCK_PAGE_SIZE = PAGE_SIZE / 2 = 0x8000
> > > 
> > > This likely fails the alignment test, returning -EINVAL instead:
> > > 
> > > # iommufd.c:988:reserved_overflow:Expected 28 (28) == errno (22)
> > > # reserved_overflow: Test failed
> > > #          FAIL  iommufd_ioas.mock_domain_limit.reserved_overflow
> > > 
> > > So, I think we'd have to pick a number aligned to MOCK_PAGE_SIZE?
> > > e.g. changing to 0x18000 for example can pass.
> > 
> > I realized that we can't change the number as it won't reproduce
> > on PAGE_SIZE=4K. So, perhaps it should just SKIP other page sizes
> > than 4K.
> 
> Shouldn't it work to parametrise both numbers accordingly, e.g. (if my
> Friday-evening maths holds up) reserve "1UL - MOCK_PAGE_SIZE * 16" then map
> "MOCK_PAGE_SIZE * 10"?

Ah, you are right, I forgot to change the reserved range.

0xfffffffffff80001 and 0x50000 could repro with 64K pages,
exactly what your math works :)

Thanks
Nicolin

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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-18 18:23       ` Robin Murphy
  2025-07-18 18:50         ` Nicolin Chen
@ 2025-07-18 19:56         ` Jason Gunthorpe
  1 sibling, 0 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2025-07-18 19:56 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Nicolin Chen, iommu, Joerg Roedel, Kevin Tian, linux-kselftest,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Fri, Jul 18, 2025 at 07:23:25PM +0100, Robin Murphy wrote:

> Shouldn't it work to parametrise both numbers accordingly, e.g. (if my
> Friday-evening maths holds up) reserve "1UL - MOCK_PAGE_SIZE * 16" then map
> "MOCK_PAGE_SIZE * 10"?

It could be done, but I wanted to capture the syzkaller test case
exactly as it was, weird randomized numbers and all.

Jason

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

* Re: [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX
  2025-07-18 18:50         ` Nicolin Chen
@ 2025-07-18 20:16           ` Jason Gunthorpe
  0 siblings, 0 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2025-07-18 20:16 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Robin Murphy, iommu, Joerg Roedel, Kevin Tian, linux-kselftest,
	Shuah Khan, Will Deacon, Lixiao Yang, Matthew Rosato, patches,
	stable, syzbot+c2f65e2801743ca64e08, Yi Liu

On Fri, Jul 18, 2025 at 11:50:30AM -0700, Nicolin Chen wrote:

> 0xfffffffffff80001 and 0x50000 could repro with 64K pages,
> exactly what your math works :)

Okay, I was also worried we'd get into trouble with how iova_alignment
might be working but if it's fine lets do this then:

@@ -975,18 +975,24 @@ TEST_F(iommufd_ioas, reserved_overflow)
                .size = sizeof(test_cmd),
                .op = IOMMU_TEST_OP_ADD_RESERVED,
                .id = self->ioas_id,
-               .add_reserved = { .start = 6,
-                                 .length = 0xffffffffffff8001 },
+               .add_reserved.start = 6,
        };
+       unsigned int map_len;
        __u64 iova;
 
-       if (PAGE_SIZE != 4096)
-               SKIP(return, "Test requires 4k PAGE_SIZE");
+       if (PAGE_SIZE == 4096) {
+               test_cmd.add_reserved.length = 0xffffffffffff8001;
+               map_len = 0x5000;
+       } else {
+               test_cmd.add_reserved.length =
+                       0xffffffffffffffff - MOCK_PAGE_SIZE * 16;
+               map_len = MOCK_PAGE_SIZE * 10;
+       }
 
        ASSERT_EQ(0,
                  ioctl(self->fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_ADD_RESERVED),
                        &test_cmd));
-       test_err_ioctl_ioas_map(ENOSPC, buffer, 0x5000, &iova);
+       test_err_ioctl_ioas_map(ENOSPC, buffer, map_len, &iova);
 }
 
 TEST_F(iommufd_ioas, area_allowed)

Thanks,
Jason

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

end of thread, other threads:[~2025-07-18 20:16 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17 19:15 [PATCH 0/2] Fix undetected overflow when allocating IOVA Jason Gunthorpe
2025-07-17 19:15 ` [PATCH 1/2] iommufd: Prevent ALIGN() overflow Jason Gunthorpe
2025-07-18  8:16   ` Nicolin Chen
2025-07-18 13:03   ` Yi Liu
2025-07-17 19:15 ` [PATCH 2/2] iommufd/selftest: Test reserved regions near ULONG_MAX Jason Gunthorpe
2025-07-17 19:15   ` kernel test robot
2025-07-18  2:45   ` Nicolin Chen
2025-07-18  8:13     ` Nicolin Chen
2025-07-18 16:21       ` Jason Gunthorpe
2025-07-18 18:23       ` Robin Murphy
2025-07-18 18:50         ` Nicolin Chen
2025-07-18 20:16           ` Jason Gunthorpe
2025-07-18 19:56         ` Jason Gunthorpe
2025-07-18 13:03   ` Yi Liu
2025-07-18 18:10 ` [PATCH 0/2] Fix undetected overflow when allocating IOVA Nicolin Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.