Re: [PATCH v1 2/2] selftest/mseal: Add tests for madvise fixes

On Thu, Oct 17, 2024 at 12:51:05AM +0000, jeffxu@chromium.org wrote:

From: Jeff Xu jeffxu@chromium.org

Testcase for "Two fixes for madvise(MADV_DONTNEED) when sealed"

test_seal_madvise_prot_none shall not block when mapping is PROT_NONE

test_madvise_filebacked_writable shall not block writeable private filebacked mapping.

test_madvise_filebacked_was_writable - shall block. shall block read-only private filebacked mapping which was previously writable.

Fixes: 8be7258aad44 ("mseal: add mseal syscall") Cc: stable@vger.kernel.org # 6.11.y: 67203f3f2a63: selftests/mm: add mseal test for no-discard madvise Cc: stable@vger.kernel.org # 6.11.y: f28bdd1b17ec: selftests/mm: add more mseal traversal tests Cc: stable@vger.kernel.org # 6.11.y Signed-off-by: Jeff Xu jeffxu@chromium.org

---

tools/testing/selftests/mm/mseal_test.c | 118 +++++++++++++++++++++++- 1 file changed, 116 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c index 01675c412b2a..fa74dbe4a684 100644 --- a/tools/testing/selftests/mm/mseal_test.c +++ b/tools/testing/selftests/mm/mseal_test.c @@ -1728,7 +1728,7 @@ static void test_seal_discard_ro_anon_on_filebacked(bool seal) FAIL_TEST_IF_FALSE(!ret); }

• /* sealing doesn't apply for file backed mapping. */

• /* read-only private file-backed mapping, allow always */ ret = sys_madvise(ptr, size, MADV_DONTNEED); FAIL_TEST_IF_FALSE(!ret);

@@ -1864,6 +1864,111 @@ static void test_seal_madvise_nodiscard(bool seal) REPORT_TEST_PASS(); }

Again you're doing things in these tests that have come up again and agin in review, and so at some point as a reviewer you just have to stop until the review-ee starts listening.

Either provide an utterly profound justification for:

FAIL_TEST_IF_FALSE(<negation>)

Or stop doing.

Either provide an utterly profound justification for:

(void *)-1 vs. MAP_FAILED

Or stop doing it.

I simply won't review/accept your test code until you do.

Thanks.

+static void test_seal_madvise_prot_none(bool seal) +{

• void *ptr;
• unsigned long page_size = getpagesize();
• unsigned long size = 4 * page_size;
• int ret;
• setup_single_address(size, &ptr);
• FAIL_TEST_IF_FALSE(ptr != (void *)-1);
• ret = sys_mprotect(ptr + page_size, page_size, PROT_NONE);
• FAIL_TEST_IF_FALSE(!ret);
• if (seal) {

• ret = seal_single_address(ptr + page_size, page_size);
  

• FAIL_TEST_IF_FALSE(!ret);
  

• }
• /* madvise(DONTNEED) should pass on PROT_NONE sealed VMA */
• ret = sys_madvise(ptr + page_size, page_size, MADV_DONTNEED);
• FAIL_TEST_IF_FALSE(!ret);
• REPORT_TEST_PASS();

+}

+static void test_madvise_filebacked_writable(bool seal) +{

• void *ptr;
• unsigned long page_size = getpagesize();
• unsigned long size = 4 * page_size;
• int ret;
• int fd;
• unsigned long mapflags = MAP_PRIVATE;
• fd = memfd_create("test", 0);
• FAIL_TEST_IF_FALSE(fd > 0);
• ret = fallocate(fd, 0, 0, size);
• FAIL_TEST_IF_FALSE(!ret);
• ptr = mmap(NULL, size, PROT_READ|PROT_WRITE, mapflags, fd, 0);
• FAIL_TEST_IF_FALSE(ptr != MAP_FAILED);
• if (seal) {

• ret = sys_mseal(ptr, size);
  

• FAIL_TEST_IF_FALSE(!ret);
  

• }
• /* sealing doesn't apply for writeable file-backed mapping. */
• ret = sys_madvise(ptr, size, MADV_DONTNEED);
• FAIL_TEST_IF_FALSE(!ret);
• ret = sys_munmap(ptr, size);
• if (seal)

• FAIL_TEST_IF_FALSE(ret < 0);
  

• else

• FAIL_TEST_IF_FALSE(!ret);
  

• close(fd);
• REPORT_TEST_PASS();

+}

+static void test_madvise_filebacked_was_writable(bool seal) +{

• void *ptr;
• unsigned long page_size = getpagesize();
• unsigned long size = 4 * page_size;
• int ret;
• int fd;
• unsigned long mapflags = MAP_PRIVATE;
• fd = memfd_create("test", 0);
• FAIL_TEST_IF_FALSE(fd > 0);
• ret = fallocate(fd, 0, 0, size);
• FAIL_TEST_IF_FALSE(!ret);
• ptr = mmap(NULL, size, PROT_READ|PROT_WRITE, mapflags, fd, 0);
• FAIL_TEST_IF_FALSE(ptr != MAP_FAILED);
• ret = sys_mprotect(ptr, size, PROT_READ);
• FAIL_TEST_IF_FALSE(!ret);
• if (seal) {

• ret = sys_mseal(ptr, size);
  

• FAIL_TEST_IF_FALSE(!ret);
  

• }
• /* read-only file-backed mapping, was writable. */
• ret = sys_madvise(ptr, size, MADV_DONTNEED);
• if (seal)

• FAIL_TEST_IF_FALSE(ret < 0);
  

• else

• FAIL_TEST_IF_FALSE(!ret);
  

• ret = sys_munmap(ptr, size);
• if (seal)

• FAIL_TEST_IF_FALSE(ret < 0);
  

• else

• FAIL_TEST_IF_FALSE(!ret);
  

• close(fd);
• REPORT_TEST_PASS();

+}

int main(int argc, char **argv) { bool test_seal = seal_support(); @@ -1876,7 +1981,7 @@ int main(int argc, char **argv) if (!pkey_supported()) ksft_print_msg("PKEY not supported\n");

• ksft_set_plan(88);

• ksft_set_plan(94);

  test_seal_addseal(); test_seal_unmapped_start();

@@ -1985,5 +2090,14 @@ int main(int argc, char **argv) test_seal_discard_ro_anon_on_pkey(false); test_seal_discard_ro_anon_on_pkey(true);

• test_seal_madvise_prot_none(false);
• test_seal_madvise_prot_none(true);
• test_madvise_filebacked_writable(false);
• test_madvise_filebacked_writable(true);
• test_madvise_filebacked_was_writable(false);
• test_madvise_filebacked_was_writable(true);
• ksft_finished();

}

2.47.0.rc1.288.g06298d1525-goog