linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH rc v2 0/4] Fix iommufd selftest FAIL and warnings with v6.16
@ 2025-06-24 18:00 Nicolin Chen
  2025-06-24 18:00 ` [PATCH rc v2 1/4] iommufd/selftest: Fix iommufd_dirty_tracking with large hugepage sizes Nicolin Chen
                   ` (4 more replies)
  0 siblings, 5 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

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

 tools/testing/selftests/iommu/iommufd_utils.h |  9 ++++-
 tools/testing/selftests/iommu/iommufd.c       | 40 ++++++++++++++-----
 2 files changed, 36 insertions(+), 13 deletions(-)

-- 
2.43.0


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

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

end of thread, other threads:[~2025-06-26 15:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [PATCH rc v2 3/4] iommufd/selftest: Add asserts testing global mfd 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

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