Message-ID: <e81dba68-ee9b-42ea-911a-4eca907c8f6a@lucifer.local>
Date: Thu, 2 Jan 2025 17:30:40 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: jeffxu@...omium.org
Cc: akpm@...ux-foundation.org, vbabka@...e.cz, Liam.Howlett@...cle.com,
        broonie@...nel.org, skhan@...uxfoundation.org,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-mm@...ck.org,
        jorgelo@...omium.org, keescook@...omium.org, pedro.falcato@...il.com,
        rdunlap@...radead.org, jannh@...gle.com
Subject: Re: [RFC PATCH v1 1/1] selftest/mm: refactor mseal_test

Sorry for delay, was super busy leading up to xmas break, then had ~2.5
weeks off.

And happy new year! :)

On Wed, Dec 11, 2024 at 05:33:11AM +0000, jeffxu@...omium.org wrote:
> From: Jeff Xu <jeffxu@...omium.org>
>
> This change creates the initial version of memorysealing.c.
>
> The introduction of memorysealing.c, which replaces mseal_test.c and

I mean I don't want to be a pain but I would kinda prefer to have 'mseal'
everywhere mseal is to avoid confusion vs. memfd seals.

Of course we in the kernel absolutely _love_ to overload the meaning of terms
but some traditions are worth breaking :)

> uses the kselftest_harness, aims to initiate a discussion on using the
> selftest harness for memory sealing tests. Upon approval of this
> approach, the migration of tests from mseal_test.c to memorysealing.c
> can be implemented in a step-by-step manner.

Well, I for one like this approach so (unsurprisingly) :) but perhaps
sensible to do it iteratively so we can also tweak things as we go.

>
> This tests addresses following feedback from previous reviews:
>
> 1> Use kselftest_harness instead of custom macro, such as EXPECT_XX,
> ASSERT_XX, etc.  (Lorenzo Stoakes, Mark Brown) [1]
>
> 2> Use MAP_FAILED to check the return of mmap (Lorenzo Stoakes).

Thanks!

>
> 3>  Adding a check for vma size and prot bits. The discussion for
>     this can be found in [2] [3], here is a brief summary:
>     This is to follow up on Pedro’s in-loop change (from
>     can_modify_mm to can_modify_vma). When mseal_test is initially
>     created, they have a common pattern:  setup memory layout,
>     seal the memory, perform a few mm-api steps, verify return code
>     (not zero).  Because of the nature of out-of-loop,  it is sufficient
>     to just verify the error code in a few cases.
>
>     With Pedro's in-loop change, the sealing check happens later in the
>     stack, thus there are more things and scenarios to verify. And there
>     was feedbacks to me that mseal_test should be extensive enough to
>     discover all regressions. Hence I'm adding check for vma size and prot
>     bits.
>
> In this change: we created two fixtures:
>
> Fixture basic:   This creates a single VMA, the VMA has a
> PROT_NONE page at each end to prevent auto-merging.
>
> Fixture wo_vma: Two VMAs back to end, a PROT_NONE page at each
> end to prevent auto-merging.
>
> In addition, I add one test (mprotec) in each fixture to demo the tests.
>
> [1] https://lore.kernel.org/all/20240830180237.1220027-5-jeffxu@chromium.org/
> [2] https://lore.kernel.org/all/CABi2SkUgDZtJtRJe+J9UNdtZn=EQzZcbMB685P=1rR7DUhg=6Q@mail.gmail.com/
> [3] https://lore.kernel.org/all/2qywbjb5ebtgwkh354w3lj3vhaothvubjokxq5fhyri5jeeton@duqngzo3swjz/
>
> Signed-off-by: Jeff Xu <jeffxu@...omium.org>
> ---
>  tools/testing/selftests/mm/.gitignore      |   1 +
>  tools/testing/selftests/mm/Makefile        |   1 +
>  tools/testing/selftests/mm/memorysealing.c | 182 +++++++++++++++++++++
>  tools/testing/selftests/mm/memorysealing.h | 116 +++++++++++++
>  tools/testing/selftests/mm/mseal_test.c    |  67 +-------
>  5 files changed, 301 insertions(+), 66 deletions(-)
>  create mode 100644 tools/testing/selftests/mm/memorysealing.c
>  create mode 100644 tools/testing/selftests/mm/memorysealing.h
>
> diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
> index a51a947b2d1d..982234a99f20 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -57,3 +57,4 @@ hugetlb_dio
>  pkey_sighandler_tests_32
>  pkey_sighandler_tests_64
>  guard-pages
> +memorysealing
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 93a46ac633df..08876624f46d 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -67,6 +67,7 @@ TEST_GEN_FILES += map_populate
>  ifneq (,$(filter $(ARCH),arm64 riscv riscv64 x86 x86_64))
>  TEST_GEN_FILES += memfd_secret
>  endif
> +TEST_GEN_FILES += memorysealing
>  TEST_GEN_FILES += migration
>  TEST_GEN_FILES += mkdirty
>  TEST_GEN_FILES += mlock-random-test
> diff --git a/tools/testing/selftests/mm/memorysealing.c b/tools/testing/selftests/mm/memorysealing.c
> new file mode 100644
> index 000000000000..e10032528b64
> --- /dev/null
> +++ b/tools/testing/selftests/mm/memorysealing.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#define _GNU_SOURCE
> +#include "../kselftest_harness.h"
> +#include <asm-generic/unistd.h>
> +#include <errno.h>
> +#include <syscall.h>
> +#include "memorysealing.h"
> +
> +/*
> + * To avoid auto-merging, create a VMA with PROT_NONE pages at each end.
> + * If unsuccessful, return MAP_FAILED.
> + */
> +static void *setup_single_address(int size, int prot)

Nitty, but size should really be an unsigned long.

> +{
> +	int ret;
> +	void *ptr;
> +	unsigned long page_size = getpagesize();

Trivial, but we store page size in self->page_size so why not pass it as a
parameter instead of looking up?

> +
> +	ptr = mmap(NULL, size + 2 * page_size, prot,
> +		MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +
> +	if (ptr == MAP_FAILED)
> +		return MAP_FAILED;
> +
> +	/* To avoid auto-merging, change to PROT_NONE at each end. */
> +	ret = sys_mprotect(ptr, page_size, PROT_NONE);
> +	if (ret != 0)
> +		return MAP_FAILED;
> +
> +	ret = sys_mprotect(ptr + size + page_size, page_size, PROT_NONE);
> +	if (ret != 0)
> +		return MAP_FAILED;
> +
> +	return ptr + page_size;
> +}

This could be done more easily with a single PROT_NONE, like:

	static void *setup_single_address(unsigned long size, int prot,
			unsigned long page_size)
	{
		void *ptr;

		/* Ensure we only merge what we intend to. */
		ptr = mmap(NULL, size + 2 * page_size, PROT_NONE,
			   MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
		if (ptr == MAP_FAILED)
			return MAP_FAILED;

		return mmap(&ptr[page_size], size, prot,
			    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
	}

It's probably worth having an equivalent freeing function to avoid leaks:

	static int free_single_address(void *ptr, unsigned long size,
			unsigned long page_size)
	{
		/* Also free the PROT_NONE sentinels. */
		return munmap(ptr - page_size, size + 2 * page_size);
	}

> +
> +FIXTURE(basic)
> +{
> +	unsigned long page_size;
> +	unsigned long size;
> +	void *ptr;
> +};
> +
> +/*
> + * Setup for basic:
> + * Single VMA with 4 pages, prot = (PROT_READ | PROT_WRITE).
> + */
> +FIXTURE_SETUP(basic)
> +{
> +	int prot;
> +
> +	self->page_size = getpagesize();
> +
> +	if (!mseal_supported())
> +		SKIP(return, "mseal is not supported");
> +
> +	/* Create a single VMA with 4 pages, prot as PROT_READ | PROT_WRITE. */
> +	self->size = self->page_size * 4;
> +	self->ptr = setup_single_address(self->size, PROT_READ | PROT_WRITE);
> +	EXPECT_NE(self->ptr, MAP_FAILED);
> +
> +	EXPECT_EQ(self->size, get_vma_size(self->ptr, &prot));

I mean fine but this check seems a bit overkill here, we don't need to
check to ensure that PROT_NONE works.

> +	EXPECT_EQ(PROT_READ | PROT_WRITE, prot);

Nitty, but this seems a bit 'Yoda style', that is placing the expectation
prior to the value whose expectation we are assessing, that is perhaps it'd
be clearer as:

	EXPECT_EQ(prot, PROT_READ | PROT_WRITE);

And again, I think this is really overkill here, if the kernel is unable to
do an ordinary mapping correctly we have bigger problems than an mseal bug!

> +};
> +
> +FIXTURE_TEARDOWN(basic)
> +{

Probably worth adding munmap() teardown here.

> +}
> +
> +FIXTURE(two_vma)
> +{
> +	unsigned long page_size;
> +	unsigned long size;
> +	void *ptr;
> +};
> +
> +/*
> + * Setup for two_vma:
> + * Two consecutive VMAs, each with 2 pages.
> + * The first VMA:  prot = PROT_READ.
> + * The second VMA: prot = (PROT_READ | PROT_WRITE).
> + */
> +FIXTURE_SETUP(two_vma)
> +{
> +	int prot;
> +	int ret;
> +
> +	self->page_size = getpagesize();
> +
> +	if (!mseal_supported())
> +		SKIP(return, "mseal is not supported");
> +
> +	/* Create a single VMA with 4 pages, prot as PROT_READ | PROT_WRITE. */
> +	self->size = getpagesize() * 4;
> +	self->ptr = setup_single_address(self->size, PROT_READ | PROT_WRITE);
> +	EXPECT_NE(self->ptr, MAP_FAILED);
> +
> +	/* Use mprotect to split as two VMA. */
> +	ret = sys_mprotect(self->ptr, self->page_size * 2, PROT_READ);
> +	ASSERT_EQ(ret, 0);
> +

> +	/* Verify the first VMA is 2 pages and prot bits */
> +	EXPECT_EQ(self->page_size * 2, get_vma_size(self->ptr, &prot));
> +	EXPECT_EQ(PROT_READ, prot);
> +
> +	/* Verify the second VMA is 2 pages and prot bits */
> +	EXPECT_EQ(self->page_size * 2,
> +		get_vma_size(self->ptr + self->page_size * 2, &prot));
> +	EXPECT_EQ(PROT_READ | PROT_WRITE, prot);

Again, as with the prior fixture, it seems a bit overkill, as this is
essentially testing 'does mmap() and mprotect()' work. We should hope so :)

> +};
> +
> +FIXTURE_TEARDOWN(two_vma)
> +{

Same comment as for the basic fixture - let's clean up after ourselves.

> +}
> +
> +/*
> + * Verify mprotect is blocked.
> + */
> +TEST_F(basic, mprotect_basic)
> +{
> +	int ret;
> +	unsigned long size;
> +	int prot;
> +
> +	/* Seal the mapping. */
> +	ret = sys_mseal(self->ptr, self->size, 0);
> +	ASSERT_EQ(ret, 0);
> +
> +	/* Verify mprotect is blocked. */
> +	ret = sys_mprotect(self->ptr, self->size, PROT_READ);
> +	EXPECT_GT(0, ret);

Yeah this one is kinda egregious, this is genuinely hard to read. Wouldn't
this be better as:

	EXPECT_NE(ret, 0);

As it explicitly says 'not equal to a success case'. Rather than 'wait 0 is
_greater than_ the result which means the result is _less than_ 0 which
means an error but let me check the man page can it be positive, oh ok it's
-1 on error, 0 on success', which was the Lorenzo thought process... :)

> +	EXPECT_EQ(EPERM, errno);

Let's invert these, Yoda style isn't really useful here and is a little
harder to read, e.g. we should do:

	EXPECT_EQ(errno, EPERM);

> +
> +	/* Verify the VMA (sealed) isn't changed */
> +	size = get_vma_size(self->ptr, &prot);
> +	EXPECT_EQ(self->size, size);
> +	EXPECT_EQ(PROT_READ | PROT_WRITE, prot);

Again, no to Yoda :)

I also wonder how useful this is - I guess belts + braces, and linux _can_
do partial failures, but is that something we would care about in practice?

> +}
> +
> +/*
> + * Seal both VMAs in one mseal call.
> + * Verify mprotect is blocked on both VMAs in various cases.
> + */
> +TEST_F(two_vma, mprotect)

I'm guessing you aren't necessarily covering _all_ mprotect cases here
(though perhaps you intend to later, iteratively?)

> +{
> +	int ret;
> +	int prot;
> +	unsigned long size;

I think 'size' is a bit confusing as it refers to the size ascertained from
the get_vma_size() later on. Perhaps call it actual_size?

Maybe worth adding:

	void *ptr = self->ptr;
	unsigned long size = self->size;
	unsigned long page_size = self->page_size;

For convenience?

This is totally optional though, maybe more effort than it's worth.

> +
> +	/* Seal both VMAs in one mseal call. */
> +	ret = sys_mseal(self->ptr, self->size, 0);
> +	ASSERT_EQ(ret, 0);
> +
> +	/* Verify mprotect is rejected on the first VMA. */
> +	ret = sys_mprotect(self->ptr, self->page_size * 2,
> +		PROT_READ | PROT_EXEC);

Why PROT_READ | PROT_EXEC? Is this arbitrary? To be thorough shouldn't you
loop through all of the prot flags that you think should fail? And perhaps
check what happens if you mprotect() the VMA to set it to what it already
is?

> +	EXPECT_GT(0, ret);

Please convert to EXPECT_NE(ret, 0)

> +	EXPECT_EQ(EPERM, errno);

Please invert to EXPECT_EQ(errno, EPERM)

> +
> +	/* Verify mprotect is rejected on the second VMA. */
> +	ret = sys_mprotect(self->ptr, self->page_size * 2,
> +		PROT_READ | PROT_EXEC);

Hang on, isn't this just a repeat of the first test? Shouldn't this be:

	ret = sys_mprotect(&self->ptr[self->page_size * 2],
		self->page_size * 2, PROT_READ | PROT_EXEC);

> +	EXPECT_GT(0, ret);
> +	EXPECT_EQ(EPERM, errno);

Same comment as above re: ordering.

> +
> +	/* Attempt of mprotect two VMAs at the same call is blocked */
> +	ret = sys_mprotect(self->ptr, self->size,
> +		PROT_READ | PROT_EXEC);

Same comment about iterating through prot flags that should fail as above.

> +	EXPECT_GT(0, ret);
> +	EXPECT_EQ(EPERM, errno);

Same comment as above re: ordering.

> +
> +	/* Verify both VMAs aren't changed. */
> +	size = get_vma_size(self->ptr, &prot);
> +	EXPECT_EQ(self->page_size * 2, size);
> +	EXPECT_EQ(PROT_READ, prot);

Same comment as above re: ordering.

> +
> +	size = get_vma_size(self->ptr + self->page_size * 2, &prot);
> +	EXPECT_EQ(self->page_size * 2, size);
> +	EXPECT_EQ(PROT_READ | PROT_WRITE, prot);

Same comment as above re: ordering.

> +}
> +
> +TEST_HARNESS_MAIN
> diff --git a/tools/testing/selftests/mm/memorysealing.h b/tools/testing/selftests/mm/memorysealing.h
> new file mode 100644
> index 000000000000..aee6e6700028
> --- /dev/null
> +++ b/tools/testing/selftests/mm/memorysealing.h
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <syscall.h>
> +
> +/*
> + * Define sys_xyx to call syscall directly.
> + * This is needed because we want to avoid calling glibc and
> + * test syscall directly.

I do wonder if this is worth it, but I mean, I guess it's not too egregious
if it's wrapped like this. Same comment for other wrappers.

> + * The only exception is mmap, which _NR_mmap2 is not defined for
> + * some ARM architecture.
> + */
> +static inline int sys_mseal(void *start, size_t len, unsigned long flags)
> +{
> +	int sret;
> +
> +	errno = 0;

Why are we setting this to 0? Same comment for all sys_XXX() wrappers.

> +	sret = syscall(__NR_mseal, start, len, flags);
> +	return sret;

Nit, but this seems a bit redundant, why not just drop the sret assignment,
and simply:

	return syscall(__NR_mseal, start, len, flags);

Same comment for all sys_XXX() wrappers.

> +}
> +
> +static inline int sys_mprotect(void *ptr, size_t size, unsigned long prot)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(__NR_mprotect, ptr, size, prot);
> +	return sret;
> +}
> +

The wrappers below don't seem to be used yet, could we delay putting them
in until they are actually used?

> +static inline int sys_mprotect_pkey(void *ptr, size_t size,
> +	unsigned long orig_prot, unsigned long pkey)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(__NR_pkey_mprotect, ptr, size, orig_prot, pkey);
> +	return sret;
> +}
> +
> +static inline int sys_munmap(void *ptr, size_t size)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(__NR_munmap, ptr, size);
> +	return sret;
> +}
> +
> +static inline int sys_madvise(void *start, size_t len, int types)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(__NR_madvise, start, len, types);
> +	return sret;
> +}
> +
> +static inline 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;
> +}
> +
> +/*
> + * Parsing /proc/self/maps to get VMA's size and prot bit.
> + */
> +static unsigned long get_vma_size(void *addr, int *prot)
> +{
> +	FILE *maps;
> +	char line[256];
> +	int size = 0;
> +	uintptr_t  addr_start, addr_end;

Why not add a:

	uintptr_t addr_val = (uintptr_t)addr;

To avoid having to constantly cast below?

> +	char protstr[5];
> +	*prot = 0;

Nit but not sure there's much point in setting this to 0 even in failure
cases?

With an error code return you can avoid having to do this.

> +
> +	maps = fopen("/proc/self/maps", "r");
> +	if (!maps)
> +		return 0;

Might it be better to return an error code or something so we can identify
this case? I guess any sensible call will fail on a 0 but you're sort of
just implicitly hoping for this.

> +
> +	while (fgets(line, sizeof(line), maps)) {
> +		if (sscanf(line, "%lx-%lx %4s", &addr_start, &addr_end, protstr) == 3) {
> +			if (addr_start == (uintptr_t) addr) {

This is a Yoda comparison again, let's put what we're comparing first, and
the thing we're comparing to afterwards, e.g.

	if ((uintptr_t)addr == addr_start) ...

Also I don't think a space after the (type) cast is necesary.

> +				size = addr_end - addr_start;
> +				if (protstr[0] == 'r')
> +					*prot |= PROT_READ;
> +				if (protstr[1] == 'w')
> +					*prot |= PROT_WRITE;
> +				if (protstr[2] == 'x')
> +					*prot |= PROT_EXEC;
> +				break;
> +			}
> +		}
> +	}

On the impl: This would be neater if you used guard clauses:

	while (fgets(line, sizeof(line), maps)) {
		if (sscanf(line, "%lx-%lx %4s", &addr_start, &addr_end, protstr) != 3)
			continue;

		if (addr_val != addr_start)
			continue;

		size = addr_end - addr_start;
		if (protstr[0] == 'r')
			*prot |= PROT_READ;
		if (protstr[1] == 'w')
			*prot |= PROT_WRITE;
		if (protstr[2] == 'x')
			*prot |= PROT_EXEC;
		break;
	}

This is a lot more readable?

On doing this at all - it feels a bit crazy to check this, but I guess it
might be the only way to assert things about merge/split.

I'm not sure it's _really useful_ to do that though - merges/splits are
internal implementation details in effect (modulo mremap single VMA
requirements), and mseal is an external interface, really the check should
be around 'can forbidden operations be performed on sealed mappings'.

However, if you do really want to do this, I think it would be much better
to use the new ioctl interface for this [0] which should avoid having to do
unpleasant text wrangling :)

[0]:https://lore.kernel.org/all/20240627170900.1672542-1-andrii@kernel.org/


> +	fclose(maps);
> +	return size;

If we don't find the line we don't seem to be indicating this, again worth
being able to pass back an error?

> +}
> +
> +static inline bool mseal_supported(void)
> +{
> +	int ret;
> +	void *ptr;
> +	unsigned long page_size = getpagesize();
> +
> +	ptr = mmap(NULL, page_size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +	if (ptr == MAP_FAILED)
> +		return false;
> +
> +	ret = sys_mseal(ptr, page_size, 0);
> +	if (ret < 0)
> +		return false;

This is kind of cute, I mean it speaks to the need I think for us to be
able to expose 'is X available?' programmatically. But I digress.

I guess this is ok, as this is something that should never fail.

But we should definitely clean up after ourselves here whether this
succeeds or fails.

> +
> +	return true;
> +}
> diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
> index ad17005521a8..8dd20341de3d 100644
> --- a/tools/testing/selftests/mm/mseal_test.c
> +++ b/tools/testing/selftests/mm/mseal_test.c
> @@ -517,30 +517,6 @@ static void test_seal_twice(void)
>  	REPORT_TEST_PASS();
>  }
>
> -static void test_seal_mprotect(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);
> -
> -	if (seal) {
> -		ret = seal_single_address(ptr, size);
> -		FAIL_TEST_IF_FALSE(!ret);
> -	}
> -
> -	ret = sys_mprotect(ptr, size, PROT_READ | PROT_WRITE);
> -	if (seal)
> -		FAIL_TEST_IF_FALSE(ret < 0);
> -	else
> -		FAIL_TEST_IF_FALSE(!ret);
> -
> -	REPORT_TEST_PASS();
> -}
> -
>  static void test_seal_start_mprotect(bool seal)
>  {
>  	void *ptr;
> @@ -658,41 +634,6 @@ static void test_seal_mprotect_unalign_len_variant_2(bool seal)
>  	REPORT_TEST_PASS();
>  }
>
> -static void test_seal_mprotect_two_vma(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);
> -
> -	/* use mprotect to split */
> -	ret = sys_mprotect(ptr, page_size * 2, PROT_READ | PROT_WRITE);
> -	FAIL_TEST_IF_FALSE(!ret);
> -
> -	if (seal) {
> -		ret = seal_single_address(ptr, page_size * 4);
> -		FAIL_TEST_IF_FALSE(!ret);
> -	}
> -
> -	ret = sys_mprotect(ptr, page_size * 2, PROT_READ | PROT_WRITE);
> -	if (seal)
> -		FAIL_TEST_IF_FALSE(ret < 0);
> -	else
> -		FAIL_TEST_IF_FALSE(!ret);
> -
> -	ret = sys_mprotect(ptr + page_size * 2, page_size * 2,
> -			PROT_READ | PROT_WRITE);
> -	if (seal)
> -		FAIL_TEST_IF_FALSE(ret < 0);
> -	else
> -		FAIL_TEST_IF_FALSE(!ret);
> -
> -	REPORT_TEST_PASS();
> -}
> -
>  static void test_seal_mprotect_two_vma_with_split(bool seal)
>  {
>  	void *ptr;
> @@ -1876,7 +1817,7 @@ int main(void)
>  	if (!pkey_supported())
>  		ksft_print_msg("PKEY not supported\n");
>
> -	ksft_set_plan(88);
> +	ksft_set_plan(84);
>
>  	test_seal_addseal();
>  	test_seal_unmapped_start();
> @@ -1889,9 +1830,6 @@ int main(void)
>  	test_seal_zero_length();
>  	test_seal_twice();
>
> -	test_seal_mprotect(false);
> -	test_seal_mprotect(true);
> -
>  	test_seal_start_mprotect(false);
>  	test_seal_start_mprotect(true);
>
> @@ -1904,9 +1842,6 @@ int main(void)
>  	test_seal_mprotect_unalign_len_variant_2(false);
>  	test_seal_mprotect_unalign_len_variant_2(true);
>
> -	test_seal_mprotect_two_vma(false);
> -	test_seal_mprotect_two_vma(true);
> -
>  	test_seal_mprotect_two_vma_with_split(false);
>  	test_seal_mprotect_two_vma_with_split(true);
>
> --
> 2.47.1.613.gc27f4b7a9f-goog
>

Powered by blists - more mailing lists