The mremap test currently segfaults because mremap
does not have a NOREPLACE flag which will fail if the
remap destination address collides with an existing mapping.
The segfault is caused by the mremap call destorying the
text region mapping of the program. This patch series fixes
the segfault by sanitizing the mremap destination address
and introduces other minor fixes to the test case.
Sidhartha Kumar (4):
selftest/vm: verify mmap addr in mremap_test
selftest/vm: verify remap destination address in mremap_test
selftest/vm: support xfail in mremap_test
selftest/vm: add skip support to mremap_test
tools/testing/selftests/vm/mremap_test.c | 79 ++++++++++++++++++++++-
tools/testing/selftests/vm/run_vmtests.sh | 11 +++-
2 files changed, 85 insertions(+), 5 deletions(-)
--
2.27.0
Allow the mremap test to be skipped due to errors
such as failing to find a valid remap region and
failure to parse the mmap_min_addr sysctl.
Signed-off-by: Sidhartha Kumar <[email protected]>
---
tools/testing/selftests/vm/run_vmtests.sh | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh
index 88e15fbb5027..eae98f5de2cc 100755
--- a/tools/testing/selftests/vm/run_vmtests.sh
+++ b/tools/testing/selftests/vm/run_vmtests.sh
@@ -272,11 +272,16 @@ echo "-------------------"
echo "running mremap_test"
echo "-------------------"
./mremap_test
-if [ $? -ne 0 ]; then
+ret_val=$?
+
+if [ $ret_val -eq 0 ]; then
+ echo "[PASS]"
+elif [ $ret_val -eq $ksft_skip ]; then
+ echo "[SKIP]"
+ exitcode=$ksft_skip
+else
echo "[FAIL]"
exitcode=1
-else
- echo "[PASS]"
fi
echo "-----------------"
--
2.27.0
Avoid calling mmap with requested addresses that
are less than the system's mmap_min_addr. Running
the test as root returns EACCES when trying to map
addresses < mmap_min_addr which is not one of the
error codes for the retry condition. Add a munmap
call after an alignment check as the mappings are
retained after the retry and can reach vm.max_map_count.
Signed-off-by: Sidhartha Kumar <[email protected]>
---
tools/testing/selftests/vm/mremap_test.c | 41 +++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
index 0624d1bd71b5..58600fee4b81 100644
--- a/tools/testing/selftests/vm/mremap_test.c
+++ b/tools/testing/selftests/vm/mremap_test.c
@@ -6,6 +6,7 @@
#include <errno.h>
#include <stdlib.h>
+#include <stdio.h>
#include <string.h>
#include <sys/mman.h>
#include <time.h>
@@ -64,6 +65,35 @@ enum {
.expect_failure = should_fail \
}
+/* Returns mmap_min_addr sysctl */
+static unsigned long long get_mmap_min_addr(void)
+{
+ FILE *fp;
+ int n_matched;
+ static unsigned long long addr;
+
+ if (addr)
+ return addr;
+
+ fp = fopen("/proc/sys/vm/mmap_min_addr", "r");
+ if (fp == NULL) {
+ ksft_print_msg("Failed to open /proc/sys/vm/mmap_min_addr: %s\n",
+ strerror(errno));
+ exit(KSFT_SKIP);
+ }
+
+ n_matched = fscanf(fp, "%llu", &addr);
+ if (n_matched != 1) {
+ ksft_print_msg("Failed to read /proc/sys/vm/mmap_min_addr: %s\n",
+ strerror(errno));
+ fclose(fp);
+ exit(KSFT_SKIP);
+ }
+
+ fclose(fp);
+ return addr;
+}
+
/*
* Returns the start address of the mapping on success, else returns
* NULL on failure.
@@ -72,8 +102,15 @@ static void *get_source_mapping(struct config c)
{
unsigned long long addr = 0ULL;
void *src_addr = NULL;
+ unsigned long long mmap_min_addr;
+
+ mmap_min_addr = get_mmap_min_addr();
+
retry:
addr += c.src_alignment;
+ if (addr < mmap_min_addr)
+ goto retry;
+
src_addr = mmap((void *) addr, c.region_size, PROT_READ | PROT_WRITE,
MAP_FIXED_NOREPLACE | MAP_ANONYMOUS | MAP_SHARED,
-1, 0);
@@ -91,8 +128,10 @@ static void *get_source_mapping(struct config c)
* alignment in the tests.
*/
if (((unsigned long long) src_addr & (c.src_alignment - 1)) ||
- !((unsigned long long) src_addr & c.src_alignment))
+ !((unsigned long long) src_addr & c.src_alignment)) {
+ munmap(src_addr, c.region_size);
goto retry;
+ }
if (!src_addr)
goto error;
--
2.27.0
Because mremap does not have a NOREPLACE flag,
it can destroy existing mappings. This can
cause a segfault if regions such as text are
destroyed. Verify the requested mremap destination
address does not overlap any existing mappings
by using mmap's FIXED_NOREPLACE flag and checking
for the EEXIST error code. Keep incrementing the
destination address until a valid mapping is found
or max address is reached.
Signed-off-by: Sidhartha Kumar <[email protected]>
---
tools/testing/selftests/vm/mremap_test.c | 36 ++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
index 58600fee4b81..98e9cff34aa7 100644
--- a/tools/testing/selftests/vm/mremap_test.c
+++ b/tools/testing/selftests/vm/mremap_test.c
@@ -10,6 +10,7 @@
#include <string.h>
#include <sys/mman.h>
#include <time.h>
+#include <limits.h>
#include "../kselftest.h"
@@ -65,6 +66,34 @@ enum {
.expect_failure = should_fail \
}
+/*
+ * Returns 0 if the requested remap region overlaps with an
+ * existing mapping (e.g text, stack) else returns 1.
+ */
+static int remap_region_valid(void *addr, unsigned long long size)
+{
+ void *remap_addr = NULL;
+ int ret = 1;
+
+ if ((unsigned long long) addr > ULLONG_MAX - size) {
+ ksft_print_msg("Can't find a valid region to remap to\n");
+ exit(KSFT_SKIP);
+ }
+
+ /* Use MAP_FIXED_NOREPLACE flag to ensure region is not mapped */
+ remap_addr = mmap(addr, size, PROT_READ | PROT_WRITE,
+ MAP_FIXED_NOREPLACE | MAP_ANONYMOUS | MAP_SHARED,
+ -1, 0);
+ if (remap_addr == MAP_FAILED) {
+ if (errno == EEXIST)
+ ret = 0;
+ } else {
+ munmap(remap_addr, size);
+ }
+
+ return ret;
+}
+
/* Returns mmap_min_addr sysctl */
static unsigned long long get_mmap_min_addr(void)
{
@@ -180,6 +209,13 @@ static long long remap_region(struct config c, unsigned int threshold_mb,
if (!((unsigned long long) addr & c.dest_alignment))
addr = (void *) ((unsigned long long) addr | c.dest_alignment);
+ /* Don't destroy existing mappings unless expected to overlap */
+ while (!remap_region_valid(addr, c.region_size)) {
+ if (c.overlapping)
+ break;
+ addr += c.src_alignment;
+ }
+
clock_gettime(CLOCK_MONOTONIC, &t_start);
dest_addr = mremap(src_addr, c.region_size, c.region_size,
MREMAP_MAYMOVE|MREMAP_FIXED, (char *) addr);
--
2.27.0
Use ksft_test_result_xfail for the tests which
are expected to fail.
Signed-off-by: Sidhartha Kumar <[email protected]>
---
tools/testing/selftests/vm/mremap_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
index 98e9cff34aa7..ace9c3596ed7 100644
--- a/tools/testing/selftests/vm/mremap_test.c
+++ b/tools/testing/selftests/vm/mremap_test.c
@@ -269,7 +269,7 @@ static void run_mremap_test_case(struct test test_case, int *failures,
if (remap_time < 0) {
if (test_case.expect_failure)
- ksft_test_result_pass("%s\n\tExpected mremap failure\n",
+ ksft_test_result_xfail("%s\n\tExpected mremap failure\n",
test_case.name);
else {
ksft_test_result_fail("%s\n", test_case.name);
--
2.27.0
On 4/14/22 11:15 AM, Sidhartha Kumar wrote:
> Use ksft_test_result_xfail for the tests which
> are expected to fail.
>
Same Nit about commit log lines.
> Signed-off-by: Sidhartha Kumar <[email protected]>
> ---
> tools/testing/selftests/vm/mremap_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
> index 98e9cff34aa7..ace9c3596ed7 100644
> --- a/tools/testing/selftests/vm/mremap_test.c
> +++ b/tools/testing/selftests/vm/mremap_test.c
> @@ -269,7 +269,7 @@ static void run_mremap_test_case(struct test test_case, int *failures,
>
> if (remap_time < 0) {
> if (test_case.expect_failure)
> - ksft_test_result_pass("%s\n\tExpected mremap failure\n",
> + ksft_test_result_xfail("%s\n\tExpected mremap failure\n",
> test_case.name);
> else {
> ksft_test_result_fail("%s\n", test_case.name);
>
Thank you. Looks good to me.
Reviewed-by: Shuah Khan <[email protected]>
thanks,
-- Shuah