From: Jeff Xu jeffxu@chromium.org
the syscall remap accepts following:
sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, (void*) 0xdead0000)
when the src is sealed, the call will fail with when error code: EPERM
Also remove the dependency on glibc for mremap(), the previous version of test pass on certain version of glibc and fail in another. By calling syscall directly, we test and verify remap function in kernel directly.
Signed-off-by: Jeff Xu jeffxu@chromium.org Reported-by: Pedro Falcato pedro.falcato@gmail.com --- tools/testing/selftests/mm/mseal_test.c | 44 ++++++++++++++----------- 1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c index a818f010de47..45d299fd74ed 100644 --- a/tools/testing/selftests/mm/mseal_test.c +++ b/tools/testing/selftests/mm/mseal_test.c @@ -110,6 +110,16 @@ static int sys_madvise(void *start, size_t len, int types) return sret; }
+static void *sys_mremap(void *addr, size_t old_len, size_t new_len, + unsigned long flags, void *new_addr) +{ + void *sret; + + errno = 0; + sret = (void *) syscall(__NR_mremap, addr, old_len, new_len, flags, new_addr); + return sret; +} + static int sys_pkey_alloc(unsigned long flags, unsigned long init_val) { int ret = syscall(__NR_pkey_alloc, flags, init_val); @@ -1115,12 +1125,12 @@ static void test_seal_mremap_shrink(bool seal) }
/* shrink from 4 pages to 2 pages. */ - ret2 = mremap(ptr, size, 2 * page_size, 0, 0); + ret2 = sys_mremap(ptr, size, 2 * page_size, 0, 0); if (seal) { - FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); + FAIL_TEST_IF_FALSE(ret2 == (void *) MAP_FAILED); FAIL_TEST_IF_FALSE(errno == EPERM); } else { - FAIL_TEST_IF_FALSE(ret2 != MAP_FAILED); + FAIL_TEST_IF_FALSE(ret2 != (void *) MAP_FAILED);
}
@@ -1147,7 +1157,7 @@ static void test_seal_mremap_expand(bool seal) }
/* expand from 2 page to 4 pages. */ - ret2 = mremap(ptr, 2 * page_size, 4 * page_size, 0, 0); + ret2 = sys_mremap(ptr, 2 * page_size, 4 * page_size, 0, 0); if (seal) { FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); FAIL_TEST_IF_FALSE(errno == EPERM); @@ -1180,7 +1190,7 @@ static void test_seal_mremap_move(bool seal) }
/* move from ptr to fixed address. */ - ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_FIXED, newPtr); + ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_FIXED, newPtr); if (seal) { FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); FAIL_TEST_IF_FALSE(errno == EPERM); @@ -1299,7 +1309,7 @@ static void test_seal_mremap_shrink_fixed(bool seal) }
/* mremap to move and shrink to fixed address */ - ret2 = mremap(ptr, size, 2 * page_size, MREMAP_MAYMOVE | MREMAP_FIXED, + ret2 = sys_mremap(ptr, size, 2 * page_size, MREMAP_MAYMOVE | MREMAP_FIXED, newAddr); if (seal) { FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); @@ -1330,7 +1340,7 @@ static void test_seal_mremap_expand_fixed(bool seal) }
/* mremap to move and expand to fixed address */ - ret2 = mremap(ptr, page_size, size, MREMAP_MAYMOVE | MREMAP_FIXED, + ret2 = sys_mremap(ptr, page_size, size, MREMAP_MAYMOVE | MREMAP_FIXED, newAddr); if (seal) { FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); @@ -1361,7 +1371,7 @@ static void test_seal_mremap_move_fixed(bool seal) }
/* mremap to move to fixed address */ - ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_FIXED, newAddr); + ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_FIXED, newAddr); if (seal) { FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); FAIL_TEST_IF_FALSE(errno == EPERM); @@ -1390,14 +1400,13 @@ static void test_seal_mremap_move_fixed_zero(bool seal) /* * MREMAP_FIXED can move the mapping to zero address */ - ret2 = mremap(ptr, size, 2 * page_size, MREMAP_MAYMOVE | MREMAP_FIXED, + ret2 = sys_mremap(ptr, size, 2 * page_size, MREMAP_MAYMOVE | MREMAP_FIXED, 0); if (seal) { FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); FAIL_TEST_IF_FALSE(errno == EPERM); } else { FAIL_TEST_IF_FALSE(ret2 == 0); - }
REPORT_TEST_PASS(); @@ -1420,13 +1429,12 @@ static void test_seal_mremap_move_dontunmap(bool seal) }
/* mremap to move, and don't unmap src addr. */ - ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, 0); + ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, 0); if (seal) { FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); FAIL_TEST_IF_FALSE(errno == EPERM); } else { FAIL_TEST_IF_FALSE(ret2 != MAP_FAILED); - }
REPORT_TEST_PASS(); @@ -1449,18 +1457,16 @@ static void test_seal_mremap_move_dontunmap_anyaddr(bool seal) }
/* - * The 0xdeaddead should not have effect on dest addr + * The 0xdead0000 should not have effect on dest addr * when MREMAP_DONTUNMAP is set. */ - ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, - 0xdeaddead); + ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, + (void *) 0xdead0000); if (seal) { - FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); + FAIL_TEST_IF_FALSE(ret2 == (void *) MAP_FAILED); FAIL_TEST_IF_FALSE(errno == EPERM); } else { - FAIL_TEST_IF_FALSE(ret2 != MAP_FAILED); - FAIL_TEST_IF_FALSE((long)ret2 != 0xdeaddead); - + FAIL_TEST_IF_FALSE(ret2 == (void *) 0xdead0000); }
REPORT_TEST_PASS();
On Wed, Aug 7, 2024 at 4:35 PM jeffxu@chromium.org wrote: <snip>
/* shrink from 4 pages to 2 pages. */
ret2 = mremap(ptr, size, 2 * page_size, 0, 0);
ret2 = sys_mremap(ptr, size, 2 * page_size, 0, 0); if (seal) {
FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
FAIL_TEST_IF_FALSE(ret2 == (void *) MAP_FAILED);
MAP_FAILED is already void *
<snip>
@@ -1449,18 +1457,16 @@ static void test_seal_mremap_move_dontunmap_anyaddr(bool seal) }
/*
* The 0xdeaddead should not have effect on dest addr
* The 0xdead0000 should not have effect on dest addr * when MREMAP_DONTUNMAP is set. */
ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP,0xdeaddead);
ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP,(void *) 0xdead0000);
You still didn't explain why this test is actually needed. Why are you testing MREMAP_DONTUNMAP's hint system? This has nothing to do with mseal, you already test the MREMAP_DONTUNMAP and MREMAP_FIXED paths in other tests. You also don't know if 0xdead0000 is a valid page (hexagon for instance seems to support 256KiB and 1MiB pages, so does ppc32, and this is not something that should be hardcoded).
I'm not a fan of just throwing random flags for tests, it should be somewhat logical.
On Wed, Aug 7, 2024 at 9:38 AM Pedro Falcato pedro.falcato@gmail.com wrote:
On Wed, Aug 7, 2024 at 4:35 PM jeffxu@chromium.org wrote:
<snip> > /* shrink from 4 pages to 2 pages. */ > - ret2 = mremap(ptr, size, 2 * page_size, 0, 0); > + ret2 = sys_mremap(ptr, size, 2 * page_size, 0, 0); > if (seal) { > - FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); > + FAIL_TEST_IF_FALSE(ret2 == (void *) MAP_FAILED);
MAP_FAILED is already void *
<snip> > @@ -1449,18 +1457,16 @@ static void test_seal_mremap_move_dontunmap_anyaddr(bool seal) > } > > /* > - * The 0xdeaddead should not have effect on dest addr > + * The 0xdead0000 should not have effect on dest addr > * when MREMAP_DONTUNMAP is set. > */ > - ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, > - 0xdeaddead); > + ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, > + (void *) 0xdead0000);
You still didn't explain why this test is actually needed. Why are you testing MREMAP_DONTUNMAP's hint system?
I responded in my previous email. The test is to make sure when sealing is applied, the call fails with correct error code. I will update the comment in v2 to clarify that.
This has nothing to do with mseal, you already test the MREMAP_DONTUNMAP and MREMAP_FIXED paths in other tests.
The remap code path is quite tricky, with many flags directing the call flow. The difference might not be that obvious:
test_seal_mremap_move_dontunmap use 0 as new_addr, 0 indicates allocating a new memory. test_seal_mremap_move_dontunmap_anyaddr uses any arbitrary address as a new address.
You also don't know if 0xdead0000 is a valid page (hexagon for instance seems to support 256KiB and 1MiB pages, so does ppc32, and this is not something that should be hardcoded).
usually hardcode value is not good practice, but the point of this test is to show mremap can really relocate the mapping to an arbitrary address.
Do you have any suggestions here ? I can think of two options to choose from:
1> use 0xd0000000 2> allocate a memory then free it, reuse the ptr.
Thanks -Jeff
On Wed, Aug 7, 2024 at 11:03 AM Jeff Xu jeffxu@google.com wrote:
On Wed, Aug 7, 2024 at 9:38 AM Pedro Falcato pedro.falcato@gmail.com wrote:
On Wed, Aug 7, 2024 at 4:35 PM jeffxu@chromium.org wrote:
<snip> > /* shrink from 4 pages to 2 pages. */ > - ret2 = mremap(ptr, size, 2 * page_size, 0, 0); > + ret2 = sys_mremap(ptr, size, 2 * page_size, 0, 0); > if (seal) { > - FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED); > + FAIL_TEST_IF_FALSE(ret2 == (void *) MAP_FAILED);
MAP_FAILED is already void *
<snip> > @@ -1449,18 +1457,16 @@ static void test_seal_mremap_move_dontunmap_anyaddr(bool seal) > } > > /* > - * The 0xdeaddead should not have effect on dest addr > + * The 0xdead0000 should not have effect on dest addr > * when MREMAP_DONTUNMAP is set. > */ > - ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, > - 0xdeaddead); > + ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, > + (void *) 0xdead0000);
You still didn't explain why this test is actually needed. Why are you testing MREMAP_DONTUNMAP's hint system?
I responded in my previous email. The test is to make sure when sealing is applied, the call fails with correct error code. I will update the comment in v2 to clarify that.
This has nothing to do with mseal, you already test the MREMAP_DONTUNMAP and MREMAP_FIXED paths in other tests.
The remap code path is quite tricky, with many flags directing the call flow. The difference might not be that obvious:
test_seal_mremap_move_dontunmap use 0 as new_addr, 0 indicates allocating a new memory. test_seal_mremap_move_dontunmap_anyaddr uses any arbitrary address as a new address.
You also don't know if 0xdead0000 is a valid page (hexagon for instance seems to support 256KiB and 1MiB pages, so does ppc32, and this is not something that should be hardcoded).
usually hardcode value is not good practice, but the point of this test is to show mremap can really relocate the mapping to an arbitrary address.
Do you have any suggestions here ? I can think of two options to choose from:
1> use 0xd0000000 2> allocate a memory then free it, reuse the ptr.
I will send out V2 that addresses those comments above. Thanks
On Wed, Aug 7, 2024 at 7:03 PM Jeff Xu jeffxu@google.com wrote: <snip>
test_seal_mremap_move_dontunmap use 0 as new_addr, 0 indicates allocating a new memory. test_seal_mremap_move_dontunmap_anyaddr uses any arbitrary address as a new address.
No, MREMAP_DONTUNMAP uses the address you pass as a hint, aka you're just testing get_unmapped_area, not any mseal capability. There's no forced moving here.
You also don't know if 0xdead0000 is a valid page (hexagon for instance seems to support 256KiB and 1MiB pages, so does ppc32, and this is not something that should be hardcoded).
usually hardcode value is not good practice, but the point of this test is to show mremap can really relocate the mapping to an arbitrary address.
That's what test_seal_mremap_move_dontunmap does, no?
Do you have any suggestions here ? I can think of two options to choose from:
1> use 0xd0000000 2> allocate a memory then free it, reuse the ptr.
Personally I'd prefer 2, if you really want to keep the test. It's also a strategy used elsewhere (e.g mremap_dontunmap.c).
FWIW I don't have the mental strength to bikeshed over this any more, so please do what you think is best!
On Wed, Aug 7, 2024 at 2:20 PM Pedro Falcato pedro.falcato@gmail.com wrote:
On Wed, Aug 7, 2024 at 7:03 PM Jeff Xu jeffxu@google.com wrote:
Do you have any suggestions here ? I can think of two options to choose from:
1> use 0xd0000000 2> allocate a memory then free it, reuse the ptr.
Personally I'd prefer 2, if you really want to keep the test. It's also a strategy used elsewhere (e.g mremap_dontunmap.c).
V2 is sent with 2. Thanks for reporting this.
-Jeff
linux-kselftest-mirror@lists.linaro.org
-
Jeff Xu -
Jeff Xu -
jeffxu＠chromium.org
-
Pedro Falcato