* [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).