I checked with the original author, the mmap_FIXED test case wasn't
properly tested and fails. Currently, it maps two consecutive (non
overlapping) pages and expects the second mapping to be denied by MDWE
but these two pages have nothing to do with each other so MDWE is
actually out of the picture here.
What the test actually intended to do was to remap a virtual address
using MAP_FIXED. However, this operation unmaps the existing mapping and
creates a new one so the va is backed by a new page and MDWE is again
out of the picture, all remappings should succeed.
This patch keeps the test case to make it clear that this situation is
expected to work.
Signed-off-by: Florent Revest <[email protected]>
Fixes: 4cf1fe34fd18 ("kselftest: vm: add tests for memory-deny-write-execute")
---
tools/testing/selftests/mm/mdwe_test.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/mm/mdwe_test.c b/tools/testing/selftests/mm/mdwe_test.c
index d0954c657feb..91aa9c3099e7 100644
--- a/tools/testing/selftests/mm/mdwe_test.c
+++ b/tools/testing/selftests/mm/mdwe_test.c
@@ -168,13 +168,10 @@ TEST_F(mdwe, mmap_FIXED)
self->p = mmap(NULL, self->size, PROT_READ, self->flags, 0, 0);
ASSERT_NE(self->p, MAP_FAILED);
- p = mmap(self->p + self->size, self->size, PROT_READ | PROT_EXEC,
+ /* MAP_FIXED unmaps the existing page before mapping which is allowed */
+ p = mmap(self->p, self->size, PROT_READ | PROT_EXEC,
self->flags | MAP_FIXED, 0, 0);
- if (variant->enabled) {
- EXPECT_EQ(p, MAP_FAILED);
- } else {
- EXPECT_EQ(p, self->p);
- }
+ EXPECT_EQ(p, self->p);
}
TEST_F(mdwe, arm64_BTI)
--
2.40.1.606.ga4b1b128d6-goog
On 17.05.23 17:03, Florent Revest wrote:
> I checked with the original author, the mmap_FIXED test case wasn't
> properly tested and fails. Currently, it maps two consecutive (non
> overlapping) pages and expects the second mapping to be denied by MDWE
> but these two pages have nothing to do with each other so MDWE is
> actually out of the picture here.
>
> What the test actually intended to do was to remap a virtual address
> using MAP_FIXED. However, this operation unmaps the existing mapping and
> creates a new one so the va is backed by a new page and MDWE is again
> out of the picture, all remappings should succeed.
>
> This patch keeps the test case to make it clear that this situation is
> expected to work.
>
> Signed-off-by: Florent Revest <[email protected]>
> Fixes: 4cf1fe34fd18 ("kselftest: vm: add tests for memory-deny-write-execute")
> ---
> tools/testing/selftests/mm/mdwe_test.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/mdwe_test.c b/tools/testing/selftests/mm/mdwe_test.c
> index d0954c657feb..91aa9c3099e7 100644
> --- a/tools/testing/selftests/mm/mdwe_test.c
> +++ b/tools/testing/selftests/mm/mdwe_test.c
> @@ -168,13 +168,10 @@ TEST_F(mdwe, mmap_FIXED)
> self->p = mmap(NULL, self->size, PROT_READ, self->flags, 0, 0);
> ASSERT_NE(self->p, MAP_FAILED);
>
> - p = mmap(self->p + self->size, self->size, PROT_READ | PROT_EXEC,
> + /* MAP_FIXED unmaps the existing page before mapping which is allowed */
> + p = mmap(self->p, self->size, PROT_READ | PROT_EXEC,
> self->flags | MAP_FIXED, 0, 0);
> - if (variant->enabled) {
> - EXPECT_EQ(p, MAP_FAILED);
> - } else {
> - EXPECT_EQ(p, self->p);
> - }
> + EXPECT_EQ(p, self->p);
> }
>
> TEST_F(mdwe, arm64_BTI)
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb