mbox series

[v2,0/4] Increase mseal test coverage

Message ID 20240829214352.963001-1-jeffxu@chromium.org
Headers show
Series Increase mseal test coverage | expand

Message

Jeff Xu Aug. 29, 2024, 9:43 p.m. UTC
From: Jeff Xu <jeffxu@chromium.org>

This series increase the test coverage of mseal_test by:

Add check for vma_size, prot, and error code for existing tests.
Add more testcases for madvise, munmap, mmap and mremap to cover
sealing in different scenarios.

The increase test coverage hopefully help to prevent future regression.
It doesn't change any existing mm api's semantics, i.e. it will pass on
linux main and 6.10 branch.

Note: in order to pass this test in mm-unstable, mm-unstable must have
Liam's fix on mmap [1]

[1] https://lore.kernel.org/linux-kselftest/vyllxuh5xbqmaoyl2mselebij5ox7cseekjcvl5gmzoxxwd2he@hxi4mpjanxzt/#t

History:
V2:
- remove the mmap fix (Liam R. Howlett will fix it separately)
- Add cover letter (Lorenzo Stoakes)
- split the testcase for ease of review (Mark Brown)

V1:
- https://lore.kernel.org/linux-kselftest/20240828225522.684774-1-jeffxu@chromium.org/

Jeff Xu (4):
  selftests/mm: mseal_test, add vma size check
  selftests/mm: mseal_test add sealed madvise type
  selftests/mm: mseal_test add more tests for mmap
  selftests/mm: mseal_test add more tests for mremap

 tools/testing/selftests/mm/mseal_test.c | 829 ++++++++++++++++++++++--
 1 file changed, 762 insertions(+), 67 deletions(-)

Comments

Pedro Falcato Aug. 30, 2024, 12:31 p.m. UTC | #1
On Thu, Aug 29, 2024 at 09:43:48PM GMT, jeffxu@chromium.org wrote:
> From: Jeff Xu <jeffxu@chromium.org>
> 
> This series increase the test coverage of mseal_test by:
> 
> Add check for vma_size, prot, and error code for existing tests.
> Add more testcases for madvise, munmap, mmap and mremap to cover
> sealing in different scenarios.
> 
> The increase test coverage hopefully help to prevent future regression.
> It doesn't change any existing mm api's semantics, i.e. it will pass on
> linux main and 6.10 branch.

I do want to be clear that we shouldn't confuse "test coverage" with being unequivocally good
if it has the possibility to paint ourselves into an API corner where details that should be left
unspecified are instead set in stone (e.g do we want to test how mprotect behaves if it finds an msealed
vma midway? no, apart from the property that really matters in this case (that sealed vmas remain untouched)).

> 
> Note: in order to pass this test in mm-unstable, mm-unstable must have
> Liam's fix on mmap [1]
> 
> [1] https://lore.kernel.org/linux-kselftest/vyllxuh5xbqmaoyl2mselebij5ox7cseekjcvl5gmzoxxwd2he@hxi4mpjanxzt/#t
> 
> History:
> V2:
> - remove the mmap fix (Liam R. Howlett will fix it separately)
> - Add cover letter (Lorenzo Stoakes)
> - split the testcase for ease of review (Mark Brown)
> 
> V1:
> - https://lore.kernel.org/linux-kselftest/20240828225522.684774-1-jeffxu@chromium.org/
> 
> Jeff Xu (4):
>   selftests/mm: mseal_test, add vma size check
>   selftests/mm: mseal_test add sealed madvise type
>   selftests/mm: mseal_test add more tests for mmap
>   selftests/mm: mseal_test add more tests for mremap
>

nit: Please follow a more standard commit naming scheme like
	selftests/mm: <change description>
or
	selftests/mseal: <change description>
Jeff Xu Aug. 30, 2024, 3:45 p.m. UTC | #2
Hi Pedro

On Fri, Aug 30, 2024 at 5:45 AM Pedro Falcato <pedro.falcato@gmail.com> wrote:
>
> On Thu, Aug 29, 2024 at 09:43:49PM GMT, jeffxu@chromium.org wrote:
> > From: Jeff Xu <jeffxu@chromium.org>
> >
> > Add check for vma size, prot bits and error return.
> >
> > Signed-off-by: Jeff Xu <jeffxu@chromium.org>
> > ---
> >  tools/testing/selftests/mm/mseal_test.c | 398 ++++++++++++++++++++----
> >  1 file changed, 332 insertions(+), 66 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
> > index e7991e5fdcf3..adc646cf576c 100644
> > --- a/tools/testing/selftests/mm/mseal_test.c
> > +++ b/tools/testing/selftests/mm/mseal_test.c
> > @@ -170,18 +170,31 @@ static void set_pkey(int pkey, unsigned long pkey_value)
> >  static void setup_single_address(int size, void **ptrOut)
> >  {
> >       void *ptr;
> > +     unsigned long page_size = getpagesize();
> >
> > -     ptr = mmap(NULL, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > -     *ptrOut = ptr;
> > +     *ptrOut = (void *)-1;
> > +     ptr = mmap(NULL, size + 2 * page_size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > +     if (ptr != (void *) -1) {
>
> MAP_FAILED, not (void *) -1
>
ok.

> > +             /* add 2 page at the beginning and end to avoid auto-merge of mapping */
> > +             sys_mprotect(ptr, page_size, PROT_NONE);
> > +             sys_mprotect(ptr + size + page_size, page_size, PROT_NONE);
> > +             *ptrOut = ptr + page_size;
> > +     }
> >  }
> >
> >  static void setup_single_address_rw(int size, void **ptrOut)
> >  {
> >       void *ptr;
> >       unsigned long mapflags = MAP_ANONYMOUS | MAP_PRIVATE;
> > +     unsigned long page_size = getpagesize();
> >
> > -     ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, mapflags, -1, 0);
> > -     *ptrOut = ptr;
> > +     *ptrOut = (void *)-1;
> > +     ptr = mmap(NULL, size + 2 * page_size, PROT_READ | PROT_WRITE, mapflags, -1, 0);
> > +     if (ptr != (void *) -1) {
>
> Same here.
ok.

> > +             sys_mprotect(ptr, page_size, PROT_NONE);
> > +             sys_mprotect(ptr + size + page_size, page_size, PROT_NONE);
> > +             *ptrOut = ptr + page_size;
> > +     }
> >  }
> >
> >  static int clean_single_address(void *ptr, int size)
> > @@ -226,6 +239,21 @@ bool pkey_supported(void)
> >       return false;
> >  }
> >
> > +bool get_vma_size_supported(void)
> > +{
> > +     void *ptr;
> > +     unsigned long page_size = getpagesize();
> > +     unsigned long size = 4 * page_size;
> > +     int prot;
> > +
> > +     setup_single_address(size, &ptr);
> > +     size = get_vma_size(ptr, &prot);
> > +     if (size == 4 * page_size && prot == 0x4)
> > +             return true;
> > +
> > +     return false;
> > +}
> > +
> >  static void test_seal_addseal(void)
> >  {
> >       int ret;
> > @@ -419,11 +447,17 @@ static void test_seal_invalid_input(void)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >
> > -     setup_single_address(8 * page_size, &ptr);
> > +     setup_single_address(9 * page_size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > -     ret = clean_single_address(ptr + 4 * page_size, 4 * page_size);
> > +
> > +     ret = clean_single_address(ptr, page_size);
> >       FAIL_TEST_IF_FALSE(!ret);
> >
> > +     ret = clean_single_address(ptr + 5 * page_size, 4 * page_size);
> > +     FAIL_TEST_IF_FALSE(!ret);
> > +
> > +     ptr = ptr + page_size;
> > +
> >       /* invalid flag */
> >       ret = syscall(__NR_mseal, ptr, size, 0x20);
> >       FAIL_TEST_IF_FALSE(ret < 0);
> > @@ -523,6 +557,7 @@ static void test_seal_mprotect(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -533,9 +568,14 @@ static void test_seal_mprotect(bool seal)
> >       }
> >
> >       ret = sys_mprotect(ptr, size, PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -547,6 +587,7 @@ static void test_seal_start_mprotect(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -558,9 +599,14 @@ static void test_seal_start_mprotect(bool seal)
> >
> >       /* the first page is sealed. */
> >       ret = sys_mprotect(ptr, page_size, PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       /* pages after the first page is not sealed. */
> > @@ -577,6 +623,7 @@ static void test_seal_end_mprotect(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -593,9 +640,14 @@ static void test_seal_end_mprotect(bool seal)
> >       /* last 3 page are sealed */
> >       ret = sys_mprotect(ptr + page_size, page_size * 3,
> >                       PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr + page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 3 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -607,6 +659,7 @@ static void test_seal_mprotect_unalign_len(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -618,9 +671,14 @@ static void test_seal_mprotect_unalign_len(bool seal)
> >
> >       /* 2 pages are sealed. */
> >       ret = sys_mprotect(ptr, page_size * 2, PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       ret = sys_mprotect(ptr + page_size * 2, page_size,
> > @@ -636,6 +694,7 @@ static void test_seal_mprotect_unalign_len_variant_2(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -646,9 +705,14 @@ static void test_seal_mprotect_unalign_len_variant_2(bool seal)
> >
> >       /* 3 pages are sealed. */
> >       ret = sys_mprotect(ptr, page_size * 3, PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 3 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       ret = sys_mprotect(ptr + page_size * 3, page_size,
> > @@ -664,6 +728,7 @@ static void test_seal_mprotect_two_vma(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -678,16 +743,26 @@ static void test_seal_mprotect_two_vma(bool seal)
> >       }
> >
> >       ret = sys_mprotect(ptr, page_size * 2, PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x6);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       ret = sys_mprotect(ptr + page_size * 2, page_size * 2,
> >                       PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr + page_size * 2, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -699,6 +774,7 @@ static void test_seal_mprotect_two_vma_with_split(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -719,17 +795,27 @@ static void test_seal_mprotect_two_vma_with_split(bool seal)
> >
> >       /* the second page is sealed. */
> >       ret = sys_mprotect(ptr + page_size, page_size, PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x6);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       /* the third page is sealed. */
> >       ret = sys_mprotect(ptr + 2 * page_size, page_size,
> >                       PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr + 2 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       /* the fouth page is not sealed. */
> > @@ -746,6 +832,7 @@ static void test_seal_mprotect_partial_mprotect(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -758,9 +845,14 @@ static void test_seal_mprotect_partial_mprotect(bool seal)
> >
> >       /* mprotect first 2 page will fail, since the first page are sealed. */
> >       ret = sys_mprotect(ptr, 2 * page_size, PROT_READ | PROT_WRITE);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -783,15 +875,15 @@ static void test_seal_mprotect_partial_mprotect_tail(bool seal)
> >       }
> >
> >       ret = sys_mprotect(ptr, size, PROT_EXEC);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > -             FAIL_TEST_IF_FALSE(!ret);
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> >
> > -     if (seal) {
> > -             FAIL_TEST_IF_FALSE(get_vma_size(ptr + page_size, &prot) > 0);
> > +             size = get_vma_size(ptr + page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> >               FAIL_TEST_IF_FALSE(prot == 0x4);
> > -     }
> > +     } else
> > +             FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> >  }
> > @@ -846,6 +938,7 @@ static void test_seal_mprotect_split(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -862,16 +955,34 @@ static void test_seal_mprotect_split(bool seal)
> >
> >       /* mprotect is sealed. */
> >       ret = sys_mprotect(ptr, 2 * page_size, PROT_READ);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x6);
> > +
> > +             size = get_vma_size(ptr + page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 3 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >
> >       ret = sys_mprotect(ptr + 2 * page_size, 2 * page_size, PROT_READ);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x6);
> > +
> > +             size = get_vma_size(ptr + page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 3 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -883,6 +994,7 @@ static void test_seal_mprotect_merge(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -899,9 +1011,18 @@ static void test_seal_mprotect_merge(bool seal)
> >
> >       /* 2 pages are sealed. */
> >       ret = sys_mprotect(ptr, 2 * page_size, PROT_READ);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x6);
> > +
> > +             size = get_vma_size(ptr + page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 1 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       /* last 2 pages are not sealed. */
> > @@ -917,6 +1038,7 @@ static void test_seal_munmap(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -928,9 +1050,14 @@ static void test_seal_munmap(bool seal)
> >
> >       /* 4 pages are sealed. */
> >       ret = sys_munmap(ptr, size);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -948,6 +1075,7 @@ static void test_seal_munmap_two_vma(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -962,15 +1090,33 @@ static void test_seal_munmap_two_vma(bool seal)
> >       }
> >
> >       ret = sys_munmap(ptr, page_size * 2);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x6);
> > +
> > +             size = get_vma_size(ptr + 2 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       ret = sys_munmap(ptr + page_size, page_size * 2);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x6);
> > +
> > +             size = get_vma_size(ptr + 2 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -1018,33 +1164,75 @@ static void test_seal_munmap_partial_across_vmas(bool seal)
> >  {
> >       void *ptr;
> >       unsigned long page_size = getpagesize();
> > -     unsigned long size = 2 * page_size;
> > +     unsigned long size = 12 * page_size;
> >       int ret;
> >       int prot;
> >
> > -     /*
> > -      * Check if a partial mseal (that results in two vmas) works correctly.
> > -      * It might unmap the first, but it'll never unmap the second (msealed) vma.
> > -      */
> > -
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> >
> >       if (seal) {
> > -             ret = sys_mseal(ptr + page_size, page_size);
> > +             ret = sys_mseal(ptr + 4 * page_size, 4 * page_size);
> >               FAIL_TEST_IF_FALSE(!ret);
> >       }
> >
> > -     ret = sys_munmap(ptr, size);
> > -     if (seal)
> > +     ret = sys_munmap(ptr, 12 * page_size);
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +
> > +             size = get_vma_size(ptr + 4 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +
> > +             size = get_vma_size(ptr + 8 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> > +
> > +     ret = sys_munmap(ptr, 6 * page_size);
> >       if (seal) {
> > -             FAIL_TEST_IF_FALSE(get_vma_size(ptr + page_size, &prot) > 0);
> > +             FAIL_TEST_IF_FALSE(ret < 0);
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +
> > +             size = get_vma_size(ptr + 4 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> >               FAIL_TEST_IF_FALSE(prot == 0x4);
> > -     }
> > +
> > +             size = get_vma_size(ptr + 8 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> > +             FAIL_TEST_IF_FALSE(!ret);
> > +
> > +     ret = sys_munmap(ptr + 6 * page_size, 6 * page_size);
> > +     if (seal) {
> > +             FAIL_TEST_IF_FALSE(ret < 0);
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +
> > +             size = get_vma_size(ptr + 4 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +
> > +             size = get_vma_size(ptr + 8 * page_size, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> > +             FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> >  }
> > @@ -1074,9 +1262,11 @@ static void test_munmap_start_freed(bool seal)
> >       ret = sys_munmap(ptr, size);
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> >
> >               size = get_vma_size(ptr + page_size, &prot);
> > -             FAIL_TEST_IF_FALSE(size == page_size * 3);
> > +             FAIL_TEST_IF_FALSE(size == 3 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else {
> >               /* note: this will be OK, even the first page is */
> >               /* already unmapped. */
> > @@ -1095,6 +1285,7 @@ static void test_munmap_end_freed(bool seal)
> >       unsigned long page_size = getpagesize();
> >       unsigned long size = 4 * page_size;
> >       int ret;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1111,9 +1302,14 @@ static void test_munmap_end_freed(bool seal)
> >
> >       /* unmap all pages. */
> >       ret = sys_munmap(ptr, size);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 3 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       REPORT_TEST_PASS();
> > @@ -1144,12 +1340,15 @@ static void test_munmap_middle_freed(bool seal)
> >       ret = sys_munmap(ptr, size);
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> >
> >               size = get_vma_size(ptr, &prot);
> >               FAIL_TEST_IF_FALSE(size == page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >
> >               size = get_vma_size(ptr + page_size * 3, &prot);
> >               FAIL_TEST_IF_FALSE(size == page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else {
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> > @@ -1170,6 +1369,7 @@ static void test_seal_mremap_shrink(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1184,6 +1384,10 @@ static void test_seal_mremap_shrink(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == (void *) MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else {
> >               FAIL_TEST_IF_FALSE(ret2 != (void *) MAP_FAILED);
> >
> > @@ -1199,6 +1403,7 @@ static void test_seal_mremap_expand(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1216,6 +1421,10 @@ static void test_seal_mremap_expand(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 2 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else {
> >               FAIL_TEST_IF_FALSE(ret2 == ptr);
> >
> > @@ -1231,6 +1440,7 @@ static void test_seal_mremap_move(bool seal)
> >       unsigned long size = page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1249,10 +1459,12 @@ static void test_seal_mremap_move(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > -     } else {
> > -             FAIL_TEST_IF_FALSE(ret2 != MAP_FAILED);
> >
> > -     }
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size ==  page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> > +             FAIL_TEST_IF_FALSE(ret2 != MAP_FAILED);
> >
> >       REPORT_TEST_PASS();
> >  }
> > @@ -1264,6 +1476,7 @@ static void test_seal_mmap_overwrite_prot(bool seal)
> >       unsigned long size = page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1279,6 +1492,10 @@ static void test_seal_mmap_overwrite_prot(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else
> >               FAIL_TEST_IF_FALSE(ret2 == ptr);
> >
> > @@ -1292,6 +1509,7 @@ static void test_seal_mmap_expand(bool seal)
> >       unsigned long size = 12 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1310,6 +1528,10 @@ static void test_seal_mmap_expand(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 8 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else
> >               FAIL_TEST_IF_FALSE(ret2 == ptr);
> >
> > @@ -1323,6 +1545,7 @@ static void test_seal_mmap_shrink(bool seal)
> >       unsigned long size = 12 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1338,6 +1561,10 @@ static void test_seal_mmap_shrink(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 12 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else
> >               FAIL_TEST_IF_FALSE(ret2 == ptr);
> >
> > @@ -1352,6 +1579,7 @@ static void test_seal_mremap_shrink_fixed(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1369,6 +1597,10 @@ static void test_seal_mremap_shrink_fixed(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else
> >               FAIL_TEST_IF_FALSE(ret2 == newAddr);
> >
> > @@ -1383,6 +1615,7 @@ static void test_seal_mremap_expand_fixed(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(page_size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1400,6 +1633,10 @@ static void test_seal_mremap_expand_fixed(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(newAddr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else
> >               FAIL_TEST_IF_FALSE(ret2 == newAddr);
> >
> > @@ -1414,6 +1651,7 @@ static void test_seal_mremap_move_fixed(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1430,6 +1668,10 @@ static void test_seal_mremap_move_fixed(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(newAddr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else
> >               FAIL_TEST_IF_FALSE(ret2 == newAddr);
> >
> > @@ -1443,6 +1685,7 @@ static void test_seal_mremap_move_fixed_zero(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1460,9 +1703,12 @@ static void test_seal_mremap_move_fixed_zero(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > -     } else {
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> > +     } else
> >               FAIL_TEST_IF_FALSE(ret2 == 0);
> > -     }
> >
> >       REPORT_TEST_PASS();
> >  }
> > @@ -1474,6 +1720,7 @@ static void test_seal_mremap_move_dontunmap(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1488,6 +1735,10 @@ static void test_seal_mremap_move_dontunmap(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else {
> >               /* kernel will allocate a new address */
> >               FAIL_TEST_IF_FALSE(ret2 != MAP_FAILED);
> > @@ -1503,6 +1754,7 @@ static void test_seal_mremap_move_dontunmap_anyaddr(bool seal)
> >       unsigned long size = 4 * page_size;
> >       int ret;
> >       void *ret2;
> > +     int prot;
> >
> >       setup_single_address(size, &ptr);
> >       FAIL_TEST_IF_FALSE(ptr != (void *)-1);
> > @@ -1529,6 +1781,10 @@ static void test_seal_mremap_move_dontunmap_anyaddr(bool seal)
> >       if (seal) {
> >               FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
> >               FAIL_TEST_IF_FALSE(errno == EPERM);
> > +
> > +             size = get_vma_size(ptr, &prot);
> > +             FAIL_TEST_IF_FALSE(size == 4 * page_size);
> > +             FAIL_TEST_IF_FALSE(prot == 0x4);
> >       } else {
> >               /* remap success and return ptr2 */
> >               FAIL_TEST_IF_FALSE(ret2 ==  ptr2);
> > @@ -1690,9 +1946,10 @@ static void test_seal_discard_ro_anon_on_pkey(bool seal)
> >       /* sealing will take effect if PKRU deny write. */
> >       set_pkey(pkey, PKEY_DISABLE_WRITE);
> >       ret = sys_madvise(ptr, size, MADV_DONTNEED);
> > -     if (seal)
> > +     if (seal) {
> >               FAIL_TEST_IF_FALSE(ret < 0);
> > -     else
> > +             FAIL_TEST_IF_FALSE(errno == EPERM);
> > +     } else
> >               FAIL_TEST_IF_FALSE(!ret);
> >
> >       /* base seal still apply. */
>
> FWIW I can't review any of the above. It's still a hard to review patch with a bunch of unrelated changes
> including VMA size, random errno checks, random vma size checks, etc.
>
The EPERM is the standard error code for modifying the sealed memory.
The vma size and prot is to verify the sealed memory isn't changed
after modifying the sealed memory.

Those are all "must-check" parts of verifying the sealing. Now,
previously with out-of-loop checks, there is no need to check any of
those because once EPERM is returned, nothing will be modified. With
in-loop change, this becomes necessary.  I do not think it is
reasonable to split the case further, i.e. go to the exact same
scenario multiple times to check different attributes of sealing.

Thanks
-Jeff


> Maybe break this down in separate patches.
>
> > @@ -1876,6 +2133,15 @@ int main(int argc, char **argv)
> >       if (!pkey_supported())
> >               ksft_print_msg("PKEY not supported\n");
> >
> > +     /*
> > +      * Possible reasons:
> > +      * - unable to read /proc/pid/maps (unlikely)
> > +      * - parsing error when reading /proc/pid/maps,e.g. len is not expected.
> > +      *   Is this "TOPDOWN" mapping or format change in /proc/pid/maps ?
>
> Why do we care? I don't think running selftests without a procfs mount is supported in any way...
> Format won't change.
>
> --
> Pedro