* [PATCH rc v2 1/4] iommufd/selftest: Fix iommufd_dirty_tracking with large hugepage sizes
2025-06-24 18:00 [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16 Nicolin Chen
@ 2025-06-24 18:00 ` Nicolin Chen
2025-06-24 18:00 ` [PATCH rc v2 2/4] iommufd/selftest: Add missing close(mfd) in memfd_mmap() Nicolin Chen
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2025-06-24 18:00 UTC (permalink / raw)
To: jgg, kevin.tian
Cc: shuah, joao.m.martins, steven.sistare, iommu, linux-kselftest,
linux-kernel, thomas.weissschuh
The hugepage test cases of iommufd_dirty_tracking have the 64MB and 128MB
coverages. Both of them are smaller than the default hugepage size 512MB,
when CONFIG_PAGE_SIZE_64KB=y. However, these test cases have a variant of
using huge pages, which would mmap(MAP_HUGETLB) using these smaller sizes
than the system hugepag size. This results in the kernel aligning up the
smaller size to 512MB. If a memory was located between the upper 64/128MB
size boundary and the hugepage 512MB boundary, it would get wiped out:
https://lore.kernel.org/all/aEoUhPYIAizTLADq@nvidia.com/
Given that this aligning up behavior is well documented, we have no choice
but to allocate a hugepage aligned size to avoid this unintended wipe out.
Instead of relying on the kernel's internal force alignment, pass the same
size to posix_memalign() and map().
Also, fix the FIXTURE_TEARDOWN() misusing munmap() to free the memory from
posix_memalign(), as munmap() doesn't destroy the allocator meta data. So,
call free() instead.
Fixes: a9af47e382a4 ("iommufd/selftest: Test IOMMU_HWPT_GET_DIRTY_BITMAP")
Cc: stable@vger.kernel.org
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
tools/testing/selftests/iommu/iommufd.c | 30 +++++++++++++++++--------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index 1a8e85afe9aa..e7eb11a94034 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -2008,6 +2008,7 @@ FIXTURE_VARIANT(iommufd_dirty_tracking)
FIXTURE_SETUP(iommufd_dirty_tracking)
{
+ size_t mmap_buffer_size;
unsigned long size;
int mmap_flags;
void *vrc;
@@ -2022,22 +2023,33 @@ FIXTURE_SETUP(iommufd_dirty_tracking)
self->fd = open("/dev/iommu", O_RDWR);
ASSERT_NE(-1, self->fd);
- rc = posix_memalign(&self->buffer, HUGEPAGE_SIZE, variant->buffer_size);
- if (rc || !self->buffer) {
- SKIP(return, "Skipping buffer_size=%lu due to errno=%d",
- variant->buffer_size, rc);
- }
-
mmap_flags = MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED;
+ mmap_buffer_size = variant->buffer_size;
if (variant->hugepages) {
/*
* MAP_POPULATE will cause the kernel to fail mmap if THPs are
* not available.
*/
mmap_flags |= MAP_HUGETLB | MAP_POPULATE;
+
+ /*
+ * Allocation must be aligned to the HUGEPAGE_SIZE, because the
+ * following mmap() will automatically align the length to be a
+ * multiple of the underlying huge page size. Failing to do the
+ * same at this allocation will result in a memory overwrite by
+ * the mmap().
+ */
+ if (mmap_buffer_size < HUGEPAGE_SIZE)
+ mmap_buffer_size = HUGEPAGE_SIZE;
+ }
+
+ rc = posix_memalign(&self->buffer, HUGEPAGE_SIZE, mmap_buffer_size);
+ if (rc || !self->buffer) {
+ SKIP(return, "Skipping buffer_size=%lu due to errno=%d",
+ mmap_buffer_size, rc);
}
assert((uintptr_t)self->buffer % HUGEPAGE_SIZE == 0);
- vrc = mmap(self->buffer, variant->buffer_size, PROT_READ | PROT_WRITE,
+ vrc = mmap(self->buffer, mmap_buffer_size, PROT_READ | PROT_WRITE,
mmap_flags, -1, 0);
assert(vrc == self->buffer);
@@ -2066,8 +2078,8 @@ FIXTURE_SETUP(iommufd_dirty_tracking)
FIXTURE_TEARDOWN(iommufd_dirty_tracking)
{
- munmap(self->buffer, variant->buffer_size);
- munmap(self->bitmap, DIV_ROUND_UP(self->bitmap_size, BITS_PER_BYTE));
+ free(self->buffer);
+ free(self->bitmap);
teardown_iommufd(self->fd, _metadata);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH rc v2 2/4] iommufd/selftest: Add missing close(mfd) in memfd_mmap()
2025-06-24 18:00 [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16 Nicolin Chen
2025-06-24 18:00 ` [PATCH rc v2 1/4] iommufd/selftest: Fix iommufd_dirty_tracking with large hugepage sizes Nicolin Chen
@ 2025-06-24 18:00 ` Nicolin Chen
2025-06-24 18:00 ` [PATCH rc v2 3/4] iommufd/selftest: Add asserts testing global mfd Nicolin Chen
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2025-06-24 18:00 UTC (permalink / raw)
To: jgg, kevin.tian
Cc: shuah, joao.m.martins, steven.sistare, iommu, linux-kselftest,
linux-kernel, thomas.weissschuh
Do not forget to close mfd in the error paths, since none of the callers
would close it when ASSERT_NE(MAP_FAILED, buf) fails.
Fixes: 0bcceb1f51c7 ("iommufd: Selftest coverage for IOMMU_IOAS_MAP_FILE")
Cc: stable@vger.kernel.org
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
tools/testing/selftests/iommu/iommufd_utils.h | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/iommu/iommufd_utils.h b/tools/testing/selftests/iommu/iommufd_utils.h
index 72f6636e5d90..6e967b58acfd 100644
--- a/tools/testing/selftests/iommu/iommufd_utils.h
+++ b/tools/testing/selftests/iommu/iommufd_utils.h
@@ -60,13 +60,18 @@ static inline void *memfd_mmap(size_t length, int prot, int flags, int *mfd_p)
{
int mfd_flags = (flags & MAP_HUGETLB) ? MFD_HUGETLB : 0;
int mfd = memfd_create("buffer", mfd_flags);
+ void *buf = MAP_FAILED;
if (mfd <= 0)
return MAP_FAILED;
if (ftruncate(mfd, length))
- return MAP_FAILED;
+ goto out;
*mfd_p = mfd;
- return mmap(0, length, prot, flags, mfd, 0);
+ buf = mmap(0, length, prot, flags, mfd, 0);
+out:
+ if (buf == MAP_FAILED)
+ close(mfd);
+ return buf;
}
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH rc v2 3/4] iommufd/selftest: Add asserts testing global mfd
2025-06-24 18:00 [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16 Nicolin Chen
2025-06-24 18:00 ` [PATCH rc v2 1/4] iommufd/selftest: Fix iommufd_dirty_tracking with large hugepage sizes Nicolin Chen
2025-06-24 18:00 ` [PATCH rc v2 2/4] iommufd/selftest: Add missing close(mfd) in memfd_mmap() Nicolin Chen
@ 2025-06-24 18:00 ` Nicolin Chen
2025-06-24 18:00 ` [PATCH rc v2 4/4] iommufd/selftest: Fix build warnings due to uninitialized mfd Nicolin Chen
2025-06-26 15:04 ` [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16 Jason Gunthorpe
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2025-06-24 18:00 UTC (permalink / raw)
To: jgg, kevin.tian
Cc: shuah, joao.m.martins, steven.sistare, iommu, linux-kselftest,
linux-kernel, thomas.weissschuh
The mfd and mfd_buffer will be used in the tests directly without an extra
check. Test them in setup_sizes() to ensure they are safe to use.
Fixes: 0bcceb1f51c7 ("iommufd: Selftest coverage for IOMMU_IOAS_MAP_FILE")
Cc: stable@vger.kernel.org
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
tools/testing/selftests/iommu/iommufd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index e7eb11a94034..e61218c0537f 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -54,6 +54,8 @@ static __attribute__((constructor)) void setup_sizes(void)
mfd_buffer = memfd_mmap(BUFFER_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
&mfd);
+ assert(mfd_buffer != MAP_FAILED);
+ assert(mfd > 0);
}
FIXTURE(iommufd)
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH rc v2 4/4] iommufd/selftest: Fix build warnings due to uninitialized mfd
2025-06-24 18:00 [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16 Nicolin Chen
` (2 preceding siblings ...)
2025-06-24 18:00 ` [PATCH rc v2 3/4] iommufd/selftest: Add asserts testing global mfd Nicolin Chen
@ 2025-06-24 18:00 ` Nicolin Chen
2025-06-26 15:04 ` [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16 Jason Gunthorpe
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2025-06-24 18:00 UTC (permalink / raw)
To: jgg, kevin.tian
Cc: shuah, joao.m.martins, steven.sistare, iommu, linux-kselftest,
linux-kernel, thomas.weissschuh
Commit 869c788909b9 ("selftests: harness: Stop using setjmp()/longjmp()")
changed the harness structure. For some unknown reason, two build warnings
occur to the iommufd selftest:
iommufd.c: In function ‘wrapper_iommufd_mock_domain_all_aligns’:
iommufd.c:1807:17: warning: ‘mfd’ may be used uninitialized in this function
1807 | close(mfd);
| ^~~~~~~~~~
iommufd.c:1767:13: note: ‘mfd’ was declared here
1767 | int mfd;
| ^~~
iommufd.c: In function ‘wrapper_iommufd_mock_domain_all_aligns_copy’:
iommufd.c:1870:17: warning: ‘mfd’ may be used uninitialized in this function
1870 | close(mfd);
| ^~~~~~~~~~
iommufd.c:1819:13: note: ‘mfd’ was declared here
1819 | int mfd;
| ^~~
All the mfd have been used in the variant->file path only, so it's likely
a false alarm.
FWIW, the commit mentioned above does not cause this, yet it might affect
gcc in a certain way that resulted in the warnings. It is also found that
ading a dummy setjmp (which doesn't make sense) could mute the warnings:
https://lore.kernel.org/all/aEi8DV+ReF3v3Rlf@nvidia.com/
The job of this selftest is to catch kernel bug, while such warnings will
unlikely disrupt its role. Mute the warning by force initializing the mfd
and add an ASSERT_GT().
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
tools/testing/selftests/iommu/iommufd.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index e61218c0537f..1926ef6b40ab 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -1748,13 +1748,15 @@ TEST_F(iommufd_mock_domain, all_aligns)
unsigned int end;
uint8_t *buf;
int prot = PROT_READ | PROT_WRITE;
- int mfd;
+ int mfd = -1;
if (variant->file)
buf = memfd_mmap(buf_size, prot, MAP_SHARED, &mfd);
else
buf = mmap(0, buf_size, prot, self->mmap_flags, -1, 0);
ASSERT_NE(MAP_FAILED, buf);
+ if (variant->file)
+ ASSERT_GT(mfd, 0);
check_refs(buf, buf_size, 0);
/*
@@ -1800,13 +1802,15 @@ TEST_F(iommufd_mock_domain, all_aligns_copy)
unsigned int end;
uint8_t *buf;
int prot = PROT_READ | PROT_WRITE;
- int mfd;
+ int mfd = -1;
if (variant->file)
buf = memfd_mmap(buf_size, prot, MAP_SHARED, &mfd);
else
buf = mmap(0, buf_size, prot, self->mmap_flags, -1, 0);
ASSERT_NE(MAP_FAILED, buf);
+ if (variant->file)
+ ASSERT_GT(mfd, 0);
check_refs(buf, buf_size, 0);
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16
2025-06-24 18:00 [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16 Nicolin Chen
` (3 preceding siblings ...)
2025-06-24 18:00 ` [PATCH rc v2 4/4] iommufd/selftest: Fix build warnings due to uninitialized mfd Nicolin Chen
@ 2025-06-26 15:04 ` Jason Gunthorpe
4 siblings, 0 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2025-06-26 15:04 UTC (permalink / raw)
To: Nicolin Chen
Cc: kevin.tian, shuah, joao.m.martins, steven.sistare, iommu,
linux-kselftest, linux-kernel, thomas.weissschuh
On Tue, Jun 24, 2025 at 11:00:44AM -0700, Nicolin Chen wrote:
> A few selftest harness changes being merged to v6.16, which exposed some
> bugs and vulnerabilities in the iommufd selftest code. Fix them properly.
>
> Note that the patch fixing the build warnings at mfd is not ideal, as it
> has possibly hit some corner case in the gcc:
> https://lore.kernel.org/all/aEi8DV+ReF3v3Rlf@nvidia.com/
>
> This is on github:
> https://github.com/nicolinc/iommufd/commits/iommufd_selftest_fixes-v6.16
>
> Changelog:
> v2
> * Add "Reviewed-by" from Jason
> * Only use kfree() in the teardown()
> * Add an mmap_buffer_size for readability
> v1
> https://lore.kernel.org/all/cover.1750049883.git.nicolinc@nvidia.com/
>
> Thanks
> Nicolin
>
> Nicolin Chen (4):
> iommufd/selftest: Fix iommufd_dirty_tracking with large hugepage sizes
> iommufd/selftest: Add missing close(mfd) in memfd_mmap()
> iommufd/selftest: Add asserts testing global mfd
> iommufd/selftest: Fix build warnings due to uninitialized mfd
Applied to for-rc, thanks
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread