diff mbox series

[v1] selftests/mm: static process_madvise() wrapper for guard-pages

Message ID 20250107142937.1870478-1-ryan.roberts@arm.com (mailing list archive)
State New
Headers show
Series [v1] selftests/mm: static process_madvise() wrapper for guard-pages | expand

Commit Message

Ryan Roberts Jan. 7, 2025, 2:29 p.m. UTC
The recently introduced guard-pages mm selftest uses the
process_madvise() syscall, a wrapper for which was added to glibc v2.36.
For those of us stuck with older distributions this causes a compile
error when compiling the mm selftests. For example Ubuntu 22.04 uses
glibc 2.35, which does not have the wrapper.

To workaround the issue, let's introduce our own static
process_madvise() wrapper that uses glibc's syscall() helper.

While we are at it, add the guard-page test suite to run_vmtests.sh so
that it can be automatically run by CI systems.

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---

Applies on top of mm-unstable (f349e79bfbf3)

Thanks,
Ryan

 tools/testing/selftests/mm/guard-pages.c  | 10 ++++++++--
 tools/testing/selftests/mm/run_vmtests.sh |  5 +++++
 2 files changed, 13 insertions(+), 2 deletions(-)

--
2.43.0

Comments

Lorenzo Stoakes Jan. 7, 2025, 2:48 p.m. UTC | #1
On Tue, Jan 07, 2025 at 02:29:35PM +0000, Ryan Roberts wrote:
> The recently introduced guard-pages mm selftest uses the
> process_madvise() syscall, a wrapper for which was added to glibc v2.36.
> For those of us stuck with older distributions this causes a compile
> error when compiling the mm selftests. For example Ubuntu 22.04 uses
> glibc 2.35, which does not have the wrapper.

Ah oops! I didn't check glibc and had erroneously assumed this would be
trivially available, but perhaps using a rolling release distro has warped
my perceptions on this rather...

At any rate you're entire correct, this is very much needed, cheers!

>
> To workaround the issue, let's introduce our own static
> process_madvise() wrapper that uses glibc's syscall() helper.
>
> While we are at it, add the guard-page test suite to run_vmtests.sh so
> that it can be automatically run by CI systems.

Oops part 2... I was sure I had added it there... thanks!

>
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>
> Applies on top of mm-unstable (f349e79bfbf3)
>
> Thanks,
> Ryan
>
>  tools/testing/selftests/mm/guard-pages.c  | 10 ++++++++--
>  tools/testing/selftests/mm/run_vmtests.sh |  5 +++++
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/guard-pages.c b/tools/testing/selftests/mm/guard-pages.c
> index d8f8dee9ebbd..ece37212a8a2 100644
> --- a/tools/testing/selftests/mm/guard-pages.c
> +++ b/tools/testing/selftests/mm/guard-pages.c
> @@ -55,6 +55,12 @@ static int pidfd_open(pid_t pid, unsigned int flags)
>  	return syscall(SYS_pidfd_open, pid, flags);
>  }
>
> +static ssize_t sys_process_madvise(int pidfd, const struct iovec *iovec,
> +				   size_t n, int advice, unsigned int flags)
> +{
> +	return syscall(__NR_process_madvise, pidfd, iovec, n, advice, flags);
> +}
> +
>  /*
>   * Enable our signal catcher and try to read/write the specified buffer. The
>   * return value indicates whether the read/write succeeds without a fatal
> @@ -419,7 +425,7 @@ TEST_F(guard_pages, process_madvise)
>  	ASSERT_EQ(munmap(&ptr_region[99 * page_size], page_size), 0);
>
>  	/* Now guard in one step. */
> -	count = process_madvise(pidfd, vec, 6, MADV_GUARD_INSTALL, 0);
> +	count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_INSTALL, 0);
>
>  	/* OK we don't have permission to do this, skip. */
>  	if (count == -1 && errno == EPERM)
> @@ -440,7 +446,7 @@ TEST_F(guard_pages, process_madvise)
>  	ASSERT_FALSE(try_read_write_buf(&ptr3[19 * page_size]));
>
>  	/* Now do the same with unguard... */
> -	count = process_madvise(pidfd, vec, 6, MADV_GUARD_REMOVE, 0);
> +	count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_REMOVE, 0);
>
>  	/* ...and everything should now succeed. */
>
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index 2fc290d9430c..00c3f07ea100 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -45,6 +45,8 @@ separated by spaces:
>  	vmalloc smoke tests
>  - hmm
>  	hmm smoke tests
> +- madv_guard
> +	test madvise(2) MADV_GUARD_INSTALL and MADV_GUARD_REMOVE options
>  - madv_populate
>  	test memadvise(2) MADV_POPULATE_{READ,WRITE} options
>  - memfd_secret
> @@ -375,6 +377,9 @@ CATEGORY="mremap" run_test ./mremap_dontunmap
>
>  CATEGORY="hmm" run_test bash ./test_hmm.sh smoke
>
> +# MADV_GUARD_INSTALL and MADV_GUARD_REMOVE tests
> +CATEGORY="madv_guard" run_test ./guard-pages
> +
>  # MADV_POPULATE_READ and MADV_POPULATE_WRITE tests
>  CATEGORY="madv_populate" run_test ./madv_populate
>
> --
> 2.43.0
>
Ryan Roberts Jan. 7, 2025, 2:51 p.m. UTC | #2
On 07/01/2025 14:48, Lorenzo Stoakes wrote:
> On Tue, Jan 07, 2025 at 02:29:35PM +0000, Ryan Roberts wrote:
>> The recently introduced guard-pages mm selftest uses the
>> process_madvise() syscall, a wrapper for which was added to glibc v2.36.
>> For those of us stuck with older distributions this causes a compile
>> error when compiling the mm selftests. For example Ubuntu 22.04 uses
>> glibc 2.35, which does not have the wrapper.
> 
> Ah oops! I didn't check glibc and had erroneously assumed this would be
> trivially available, but perhaps using a rolling release distro has warped
> my perceptions on this rather...
> 
> At any rate you're entire correct, this is very much needed, cheers!
> 
>>
>> To workaround the issue, let's introduce our own static
>> process_madvise() wrapper that uses glibc's syscall() helper.
>>
>> While we are at it, add the guard-page test suite to run_vmtests.sh so
>> that it can be automatically run by CI systems.
> 
> Oops part 2... I was sure I had added it there... thanks!
> 
>>
>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

No worries, thanks for the quick review!

> 
>> ---
>>
>> Applies on top of mm-unstable (f349e79bfbf3)
>>
>> Thanks,
>> Ryan
>>
>>  tools/testing/selftests/mm/guard-pages.c  | 10 ++++++++--
>>  tools/testing/selftests/mm/run_vmtests.sh |  5 +++++
>>  2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/guard-pages.c b/tools/testing/selftests/mm/guard-pages.c
>> index d8f8dee9ebbd..ece37212a8a2 100644
>> --- a/tools/testing/selftests/mm/guard-pages.c
>> +++ b/tools/testing/selftests/mm/guard-pages.c
>> @@ -55,6 +55,12 @@ static int pidfd_open(pid_t pid, unsigned int flags)
>>  	return syscall(SYS_pidfd_open, pid, flags);
>>  }
>>
>> +static ssize_t sys_process_madvise(int pidfd, const struct iovec *iovec,
>> +				   size_t n, int advice, unsigned int flags)
>> +{
>> +	return syscall(__NR_process_madvise, pidfd, iovec, n, advice, flags);
>> +}
>> +
>>  /*
>>   * Enable our signal catcher and try to read/write the specified buffer. The
>>   * return value indicates whether the read/write succeeds without a fatal
>> @@ -419,7 +425,7 @@ TEST_F(guard_pages, process_madvise)
>>  	ASSERT_EQ(munmap(&ptr_region[99 * page_size], page_size), 0);
>>
>>  	/* Now guard in one step. */
>> -	count = process_madvise(pidfd, vec, 6, MADV_GUARD_INSTALL, 0);
>> +	count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_INSTALL, 0);
>>
>>  	/* OK we don't have permission to do this, skip. */
>>  	if (count == -1 && errno == EPERM)
>> @@ -440,7 +446,7 @@ TEST_F(guard_pages, process_madvise)
>>  	ASSERT_FALSE(try_read_write_buf(&ptr3[19 * page_size]));
>>
>>  	/* Now do the same with unguard... */
>> -	count = process_madvise(pidfd, vec, 6, MADV_GUARD_REMOVE, 0);
>> +	count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_REMOVE, 0);
>>
>>  	/* ...and everything should now succeed. */
>>
>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>> index 2fc290d9430c..00c3f07ea100 100755
>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>> @@ -45,6 +45,8 @@ separated by spaces:
>>  	vmalloc smoke tests
>>  - hmm
>>  	hmm smoke tests
>> +- madv_guard
>> +	test madvise(2) MADV_GUARD_INSTALL and MADV_GUARD_REMOVE options
>>  - madv_populate
>>  	test memadvise(2) MADV_POPULATE_{READ,WRITE} options
>>  - memfd_secret
>> @@ -375,6 +377,9 @@ CATEGORY="mremap" run_test ./mremap_dontunmap
>>
>>  CATEGORY="hmm" run_test bash ./test_hmm.sh smoke
>>
>> +# MADV_GUARD_INSTALL and MADV_GUARD_REMOVE tests
>> +CATEGORY="madv_guard" run_test ./guard-pages
>> +
>>  # MADV_POPULATE_READ and MADV_POPULATE_WRITE tests
>>  CATEGORY="madv_populate" run_test ./madv_populate
>>
>> --
>> 2.43.0
>>
diff mbox series

Patch

diff --git a/tools/testing/selftests/mm/guard-pages.c b/tools/testing/selftests/mm/guard-pages.c
index d8f8dee9ebbd..ece37212a8a2 100644
--- a/tools/testing/selftests/mm/guard-pages.c
+++ b/tools/testing/selftests/mm/guard-pages.c
@@ -55,6 +55,12 @@  static int pidfd_open(pid_t pid, unsigned int flags)
 	return syscall(SYS_pidfd_open, pid, flags);
 }

+static ssize_t sys_process_madvise(int pidfd, const struct iovec *iovec,
+				   size_t n, int advice, unsigned int flags)
+{
+	return syscall(__NR_process_madvise, pidfd, iovec, n, advice, flags);
+}
+
 /*
  * Enable our signal catcher and try to read/write the specified buffer. The
  * return value indicates whether the read/write succeeds without a fatal
@@ -419,7 +425,7 @@  TEST_F(guard_pages, process_madvise)
 	ASSERT_EQ(munmap(&ptr_region[99 * page_size], page_size), 0);

 	/* Now guard in one step. */
-	count = process_madvise(pidfd, vec, 6, MADV_GUARD_INSTALL, 0);
+	count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_INSTALL, 0);

 	/* OK we don't have permission to do this, skip. */
 	if (count == -1 && errno == EPERM)
@@ -440,7 +446,7 @@  TEST_F(guard_pages, process_madvise)
 	ASSERT_FALSE(try_read_write_buf(&ptr3[19 * page_size]));

 	/* Now do the same with unguard... */
-	count = process_madvise(pidfd, vec, 6, MADV_GUARD_REMOVE, 0);
+	count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_REMOVE, 0);

 	/* ...and everything should now succeed. */

diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index 2fc290d9430c..00c3f07ea100 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -45,6 +45,8 @@  separated by spaces:
 	vmalloc smoke tests
 - hmm
 	hmm smoke tests
+- madv_guard
+	test madvise(2) MADV_GUARD_INSTALL and MADV_GUARD_REMOVE options
 - madv_populate
 	test memadvise(2) MADV_POPULATE_{READ,WRITE} options
 - memfd_secret
@@ -375,6 +377,9 @@  CATEGORY="mremap" run_test ./mremap_dontunmap

 CATEGORY="hmm" run_test bash ./test_hmm.sh smoke

+# MADV_GUARD_INSTALL and MADV_GUARD_REMOVE tests
+CATEGORY="madv_guard" run_test ./guard-pages
+
 # MADV_POPULATE_READ and MADV_POPULATE_WRITE tests
 CATEGORY="madv_populate" run_test ./madv_populate