stable.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).