Received-SPF: pass (google.com: domain of linux-kernel+bounces-13646-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <45cc343e-f727-4cd5-b688-9a60a87875c8@collabora.com>
Date: Sun, 31 Dec 2023 11:39:43 +0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>, jeffxu@google.com,
 jorgelo@chromium.org, groeck@chromium.org, jannh@google.com,
 linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
 linux-mm@kvack.org, willy@infradead.org, pedro.falcato@gmail.com,
 keescook@chromium.org, dave.hansen@intel.com, gregkh@linuxfoundation.org,
 linux-hardening@vger.kernel.org, deraadt@openbsd.org,
 akpm@linux-foundation.org, torvalds@linux-foundation.org,
 sroettger@google.com
Subject: Re: [RFC PATCH v3 10/11] selftest mm/mseal memory sealing
To: jeffxu@chromium.org
References: <20231212231706.2680890-1-jeffxu@chromium.org>
 <20231212231706.2680890-11-jeffxu@chromium.org>
Content-Language: en-US
From: Muhammad Usama Anjum <usama.anjum@collabora.com>
In-Reply-To: <20231212231706.2680890-11-jeffxu@chromium.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

I wasn't CC-ed on the patch even though I'd reviewed the earlier revision.


On 12/13/23 4:17 AM, jeffxu@chromium.org wrote:
> From: Jeff Xu <jeffxu@chromium.org>
> 
> selftest for memory sealing change in mmap() and mseal().
> 
> Signed-off-by: Jeff Xu <jeffxu@chromium.org>
> ---
>  tools/testing/selftests/mm/.gitignore   |    1 +
>  tools/testing/selftests/mm/Makefile     |    1 +
>  tools/testing/selftests/mm/config       |    1 +
>  tools/testing/selftests/mm/mseal_test.c | 2141 +++++++++++++++++++++++
>  4 files changed, 2144 insertions(+)
>  create mode 100644 tools/testing/selftests/mm/mseal_test.c
> 
> diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
> index cdc9ce4426b9..f0f22a649985 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -43,3 +43,4 @@ mdwe_test
>  gup_longterm
>  mkdirty
>  va_high_addr_switch
> +mseal_test
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 6a9fc5693145..0c086cecc093 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -59,6 +59,7 @@ TEST_GEN_FILES += mlock2-tests
>  TEST_GEN_FILES += mrelease_test
>  TEST_GEN_FILES += mremap_dontunmap
>  TEST_GEN_FILES += mremap_test
> +TEST_GEN_FILES += mseal_test
>  TEST_GEN_FILES += on-fault-limit
>  TEST_GEN_FILES += thuge-gen
>  TEST_GEN_FILES += transhuge-stress
> diff --git a/tools/testing/selftests/mm/config b/tools/testing/selftests/mm/config
> index be087c4bc396..cf2b8780e9b1 100644
> --- a/tools/testing/selftests/mm/config
> +++ b/tools/testing/selftests/mm/config
> @@ -6,3 +6,4 @@ CONFIG_TEST_HMM=m
>  CONFIG_GUP_TEST=y
>  CONFIG_TRANSPARENT_HUGEPAGE=y
>  CONFIG_MEM_SOFT_DIRTY=y
> +CONFIG_MSEAL=y
> diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
> new file mode 100644
> index 000000000000..0692485d8b3c
> --- /dev/null
> +++ b/tools/testing/selftests/mm/mseal_test.c
> @@ -0,0 +1,2141 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <sys/mman.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <sys/time.h>
> +#include <sys/resource.h>
> +#include <stdbool.h>
> +#include "../kselftest.h"
> +#include <syscall.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <assert.h>
> +#include <fcntl.h>
> +#include <assert.h>
> +#include <sys/ioctl.h>
> +#include <sys/vfs.h>
> +#include <sys/stat.h>
> +
> +/*
> + * need those definition for manually build using gcc.
> + * gcc -I ../../../../usr/include   -DDEBUG -O3  -DDEBUG -O3 mseal_test.c -o mseal_test
> + */
> +#ifndef MM_SEAL_SEAL
> +#define MM_SEAL_SEAL 0x1
> +#endif
> +
> +#ifndef MM_SEAL_BASE
> +#define MM_SEAL_BASE 0x2
> +#endif
> +
> +#ifndef MM_SEAL_PROT_PKEY
> +#define MM_SEAL_PROT_PKEY 0x4
> +#endif
> +
> +#ifndef MM_SEAL_DISCARD_RO_ANON
> +#define MM_SEAL_DISCARD_RO_ANON 0x8
> +#endif
> +
> +#ifndef MAP_SEALABLE
> +#define MAP_SEALABLE 0x8000000
> +#endif
> +
> +#ifndef PROT_SEAL_SEAL
> +#define PROT_SEAL_SEAL 0x04000000
> +#endif
> +
> +#ifndef PROT_SEAL_BASE
> +#define PROT_SEAL_BASE 0x08000000
> +#endif
> +
> +#ifndef PROT_SEAL_PROT_PKEY
> +#define PROT_SEAL_PROT_PKEY 0x10000000
> +#endif
> +
> +#ifndef PROT_SEAL_DISCARD_RO_ANON
> +#define PROT_SEAL_DISCARD_RO_ANON 0x20000000
> +#endif
> +
> +#ifndef PKEY_DISABLE_ACCESS
> +# define PKEY_DISABLE_ACCESS    0x1
> +#endif
> +
> +#ifndef PKEY_DISABLE_WRITE
> +# define PKEY_DISABLE_WRITE     0x2
> +#endif
> +
> +#ifndef PKEY_BITS_PER_KEY
> +#define PKEY_BITS_PER_PKEY      2
> +#endif
> +
> +#ifndef PKEY_MASK
> +#define PKEY_MASK       (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE)
> +#endif
> +
> +#ifndef DEBUG
> +#define LOG_TEST_ENTER()	{}
> +#else
> +#define LOG_TEST_ENTER()	{ksft_print_msg("%s\n", __func__); }
> +#endif
> +
> +#ifndef u64
> +#define u64 unsigned long long
> +#endif
> +
> +/*
> + * define sys_xyx to call syscall directly.
> + */
> +static int sys_mseal(void *start, size_t len, int types)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(__NR_mseal, start, len, types, 0);
> +	return sret;
> +}
> +
> +int sys_mprotect(void *ptr, size_t size, unsigned long prot)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(SYS_mprotect, ptr, size, prot);
> +	return sret;
> +}
> +
> +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;
> +}
> +
> +int sys_munmap(void *ptr, size_t size)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(SYS_munmap, ptr, size);
> +	return sret;
> +}
> +
> +static int sys_madvise(void *start, size_t len, int types)
> +{
> +	int sret;
> +
> +	errno = 0;
> +	sret = syscall(__NR_madvise, start, len, types);
> +	return sret;
> +}
> +
> +int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
> +{
> +	int ret = syscall(SYS_pkey_alloc, flags, init_val);
Add empty line here.

> +	return ret;
> +}
> +
> +static inline unsigned int __read_pkey_reg(void)
> +{
> +	unsigned int eax, edx;
> +	unsigned int ecx = 0;
> +	unsigned int pkey_reg;
> +
> +	asm volatile(".byte 0x0f,0x01,0xee\n\t"
> +			: "=a" (eax), "=d" (edx)
> +			: "c" (ecx));
> +	pkey_reg = eax;
> +	return pkey_reg;
> +}
> +
> +static inline void __write_pkey_reg(u64 pkey_reg)
> +{
> +	unsigned int eax = pkey_reg;
> +	unsigned int ecx = 0;
> +	unsigned int edx = 0;
> +
> +	asm volatile(".byte 0x0f,0x01,0xef\n\t"
> +			: : "a" (eax), "c" (ecx), "d" (edx));
> +	assert(pkey_reg == __read_pkey_reg());
> +}
> +
> +static inline unsigned long pkey_bit_position(int pkey)
> +{
> +	return pkey * PKEY_BITS_PER_PKEY;
> +}
> +
> +static inline u64 set_pkey_bits(u64 reg, int pkey, u64 flags)
> +{
> +	unsigned long shift = pkey_bit_position(pkey);
> +	/* mask out bits from pkey in old value */
> +	reg &= ~((u64)PKEY_MASK << shift);
> +	/* OR in new bits for pkey */
> +	reg |= (flags & PKEY_MASK) << shift;
> +	return reg;
> +}
> +
> +static inline void set_pkey(int pkey, unsigned long pkey_value)
> +{
> +	unsigned long mask = (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);
> +	u64 new_pkey_reg;
> +
> +	assert(!(pkey_value & ~mask));
> +	new_pkey_reg = set_pkey_bits(__read_pkey_reg(), pkey, pkey_value);
> +	__write_pkey_reg(new_pkey_reg);
> +}
> +
> +void setup_single_address(int size, void **ptrOut)
> +{
> +	void *ptr;
> +
> +	ptr = mmap(NULL, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE | MAP_SEALABLE, -1, 0);
> +	assert(ptr != (void *)-1);
> +	*ptrOut = ptr;
> +}
> +
> +void setup_single_address_sealable(int size, void **ptrOut, bool sealable)
> +{
> +	void *ptr;
> +	unsigned long mapflags = MAP_ANONYMOUS | MAP_PRIVATE;
> +
> +	if (sealable)
> +		mapflags |= MAP_SEALABLE;
> +
> +	ptr = mmap(NULL, size, PROT_READ, mapflags, -1, 0);
> +	assert(ptr != (void *)-1);
> +	*ptrOut = ptr;
> +}
> +
> +void setup_single_address_rw_sealable(int size, void **ptrOut, bool sealable)
> +{
> +	void *ptr;
> +	unsigned long mapflags = MAP_ANONYMOUS | MAP_PRIVATE;
> +
> +	if (sealable)
> +		mapflags |= MAP_SEALABLE;
> +
> +	ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, mapflags, -1, 0);
> +	assert(ptr != (void *)-1);
> +	*ptrOut = ptr;
> +}
> +
> +void clean_single_address(void *ptr, int size)
> +{
> +	int ret;
> +
> +	ret = munmap(ptr, size);
> +	assert(!ret);
> +}
> +
> +void seal_mprotect_single_address(void *ptr, int size)
> +{
> +	int ret;
> +
> +	ret = sys_mseal(ptr, size, MM_SEAL_PROT_PKEY);
> +	assert(!ret);
> +}
> +
> +void seal_discard_ro_anon_single_address(void *ptr, int size)
> +{
> +	int ret;
> +
> +	ret = sys_mseal(ptr, size, MM_SEAL_DISCARD_RO_ANON);
> +	assert(!ret);
> +}
> +
> +static void test_seal_addseals(void)
> +{
> +	LOG_TEST_ENTER();
> +	int ret;
> +	void *ptr;
> +	unsigned long page_size = getpagesize();
> +	unsigned long size = 4 * page_size;
> +
> +	setup_single_address(size, &ptr);
> +
> +	/* adding seal one by one */
> +
> +	ret = sys_mseal(ptr, size, MM_SEAL_BASE);
> +	assert(!ret);
> +	ret = sys_mseal(ptr, size, MM_SEAL_PROT_PKEY);
> +	assert(!ret);
> +	ret = sys_mseal(ptr, size, MM_SEAL_SEAL);
> +	assert(!ret);
> +}
> +
> +static void test_seal_addseals_combined(void)
> +{
> +	LOG_TEST_ENTER();
> +	int ret;
> +	void *ptr;
> +	unsigned long page_size = getpagesize();
> +	unsigned long size = 4 * page_size;
> +
> +	setup_single_address(size, &ptr);
> +
> +	ret = sys_mseal(ptr, size, MM_SEAL_PROT_PKEY);
> +	assert(!ret);
> +
> +	/* adding multiple seals */
> +	ret = sys_mseal(ptr, size,
> +			MM_SEAL_PROT_PKEY | MM_SEAL_BASE|
> +			MM_SEAL_SEAL);
> +	assert(!ret);
> +
> +	/* not adding more seal type, so ok. */
> +	ret = sys_mseal(ptr, size, MM_SEAL_BASE);
> +	assert(!ret);
> +
> +	/* not adding more seal type, so ok. */
> +	ret = sys_mseal(ptr, size, MM_SEAL_SEAL);
> +	assert(!ret);
> +}
> +
> +static void test_seal_addseals_reject(void)
> +{
> +	LOG_TEST_ENTER();
> +	int ret;
> +	void *ptr;
> +	unsigned long page_size = getpagesize();
> +	unsigned long size = 4 * page_size;
> +
> +	setup_single_address(size, &ptr);
> +
> +	ret = sys_mseal(ptr, size, MM_SEAL_BASE | MM_SEAL_SEAL);
> +	assert(!ret);
> +
> +	/* MM_SEAL_SEAL is set, so not allow new seal type . */
> +	ret = sys_mseal(ptr, size,
> +			MM_SEAL_PROT_PKEY | MM_SEAL_BASE | MM_SEAL_SEAL);
> +	assert(ret < 0);
> +}
> +
> +static void test_seal_unmapped_start(void)
> +{
> +	LOG_TEST_ENTER();
> +	int ret;
> +	void *ptr;
> +	unsigned long page_size = getpagesize();
> +	unsigned long size = 4 * page_size;
> +
> +	setup_single_address(size, &ptr);
> +
> +	// munmap 2 pages from ptr.
Don't use different commenting styles in one file. Use /* */ for comments.

> +	ret = sys_munmap(ptr, 2 * page_size);
> +	assert(!ret);
> +
> +	// mprotect will fail because 2 pages from ptr are unmapped.
> +	ret = sys_mprotect(ptr, size, PROT_READ | PROT_WRITE);
> +	assert(ret < 0);
> +
> +	// mseal will fail because 2 pages from ptr are unmapped.
> +	ret = sys_mseal(ptr, size, MM_SEAL_SEAL);
> +	assert(ret < 0);
> +
> +	ret = sys_mseal(ptr + 2 * page_size, 2 * page_size, MM_SEAL_SEAL);
> +	assert(!ret);
> +}
> +