lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <be2346e4-e8c0-4470-9bf4-59eb864063a8@lucifer.local>
Date:   Sun, 28 May 2023 16:03:22 +0100
From:   Lorenzo Stoakes <lstoakes@...il.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linux-kselftest@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Shuah Khan <shuah@...nel.org>, Jens Axboe <axboe@...nel.dk>,
        Peter Xu <peterx@...hat.com>, Jason Gunthorpe <jgg@...dia.com>,
        John Hubbard <jhubbard@...dia.com>, Jan Kara <jack@...e.cz>
Subject: Re: [PATCH v1 2/3] selftests/mm: gup_longterm: new functional test
 for FOLL_LONGTERM

On Fri, May 19, 2023 at 12:27:22PM +0200, David Hildenbrand wrote:
> Let's add a new test for checking whether GUP long-term page pinning
> works as expected (R/O vs. R/W, MAP_PRIVATE vs. MAP_SHARED, GUP vs.
> GUP-fast). Note that COW handling with long-term R/O pinning in private
> mappings, and pinning of anonymous memory in general, is tested by the
> COW selftest. This test, therefore, focuses on page pinning in
> file mappings.
>
> The most interesting case is probably the "local tmpfile" case, as that
> will likely end up on a "real" filesystem such as ext4 or xfs, not on a
> virtual one like tmpfs or hugetlb where any long-term page pinning is
> always expected to succeed.
>
> For now, only add tests that use the "/sys/kernel/debug/gup_test"
> interface. We'll add tests based on liburing separately next.
>
> Signed-off-by: David Hildenbrand <david@...hat.com>
> ---
>  tools/testing/selftests/mm/Makefile       |   3 +
>  tools/testing/selftests/mm/gup_longterm.c | 386 ++++++++++++++++++++++
>  tools/testing/selftests/mm/run_vmtests.sh |   4 +-
>  3 files changed, 392 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/mm/gup_longterm.c
>
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 23af4633f0f4..95acb099315e 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -34,6 +34,7 @@ LDLIBS = -lrt -lpthread
>
>  TEST_GEN_PROGS = cow
>  TEST_GEN_PROGS += compaction_test
> +TEST_GEN_PROGS += gup_longterm
>  TEST_GEN_PROGS += gup_test
>  TEST_GEN_PROGS += hmm-tests
>  TEST_GEN_PROGS += hugetlb-madvise
> @@ -164,6 +165,8 @@ endif
>  # IOURING_EXTRA_LIBS may get set in local_config.mk, or it may be left empty.
>  $(OUTPUT)/cow: LDLIBS += $(IOURING_EXTRA_LIBS)
>
> +$(OUTPUT)/gup_longterm: LDLIBS += $(IOURING_EXTRA_LIBS)
> +
>  $(OUTPUT)/mlock-random-test $(OUTPUT)/memfd_secret: LDLIBS += -lcap
>
>  $(OUTPUT)/ksm_tests: LDLIBS += -lnuma
> diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c
> new file mode 100644
> index 000000000000..44a3617fd423
> --- /dev/null
> +++ b/tools/testing/selftests/mm/gup_longterm.c
> @@ -0,0 +1,386 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * GUP long-term page pinning tests.
> + *
> + * Copyright 2023, Red Hat, Inc.
> + *
> + * Author(s): David Hildenbrand <david@...hat.com>
> + */
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <assert.h>
> +#include <sys/mman.h>
> +#include <sys/ioctl.h>
> +#include <sys/vfs.h>
> +#include <linux/magic.h>
> +#include <linux/memfd.h>
> +
> +#include "local_config.h"
> +
> +#include "../../../../mm/gup_test.h"
> +#include "../kselftest.h"
> +#include "vm_util.h"
> +
> +static size_t pagesize;
> +static int nr_hugetlbsizes;
> +static size_t hugetlbsizes[10];
> +static int gup_fd;
> +
> +static __fsword_t get_fs_type(int fd)
> +{
> +	struct statfs fs;
> +	int ret;
> +
> +	do {
> +		ret = fstatfs(fd, &fs);
> +	} while (ret && errno == EINTR);
> +
> +	return ret ? 0 : fs.f_type;
> +}
> +
> +static bool fs_is_unknown(__fsword_t fs_type)
> +{
> +	/*
> +	 * We only support some filesystems in our tests when dealing with
> +	 * R/W long-term pinning. For these filesystems, we can be fairly sure
> +	 * whether they support it or not.
> +	 */
> +	switch (fs_type) {
> +	case TMPFS_MAGIC:
> +	case HUGETLBFS_MAGIC:
> +	case BTRFS_SUPER_MAGIC:
> +	case EXT4_SUPER_MAGIC:
> +	case XFS_SUPER_MAGIC:
> +		return false;
> +	default:
> +		return true;
> +	}
> +}
> +
> +static bool fs_supports_writable_longterm_pinning(__fsword_t fs_type)
> +{
> +	assert(!fs_is_unknown(fs_type));
> +	switch (fs_type) {
> +	case TMPFS_MAGIC:
> +	case HUGETLBFS_MAGIC:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +enum test_type {
> +	TEST_TYPE_RO,
> +	TEST_TYPE_RO_FAST,
> +	TEST_TYPE_RW,
> +	TEST_TYPE_RW_FAST,
> +};
> +
> +static void do_test(int fd, size_t size, enum test_type type, bool shared)
> +{
> +	__fsword_t fs_type = get_fs_type(fd);
> +	bool should_work;
> +	char *mem;
> +	int ret;
> +
> +	if (ftruncate(fd, size)) {
> +		ksft_test_result_fail("ftruncate() failed\n");
> +		return;
> +	}
> +
> +	if (fallocate(fd, 0, 0, size)) {
> +		if (size == pagesize)
> +			ksft_test_result_fail("fallocate() failed\n");
> +		else
> +			ksft_test_result_skip("need more free huge pages\n");
> +		return;
> +	}
> +
> +	mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
> +		   shared ? MAP_SHARED : MAP_PRIVATE, fd, 0);
> +	if (mem == MAP_FAILED) {
> +		if (size == pagesize || shared)
> +			ksft_test_result_fail("mmap() failed\n");
> +		else
> +			ksft_test_result_skip("need more free huge pages\n");
> +		return;
> +	}
> +
> +	/*
> +	 * Fault in the page writable such that GUP-fast can eventually pin
> +	 * it immediately.
> +	 */
> +	memset(mem, 0, size);

Nitty but why not just MAP_POPULATE?

> +
> +	switch (type) {
> +	case TEST_TYPE_RO:
> +	case TEST_TYPE_RO_FAST:
> +	case TEST_TYPE_RW:
> +	case TEST_TYPE_RW_FAST: {
> +		struct pin_longterm_test args;
> +		const bool fast = type == TEST_TYPE_RO_FAST ||
> +				  type == TEST_TYPE_RW_FAST;
 +		const bool rw = type == TEST_TYPE_RW ||
> +				type == TEST_TYPE_RW_FAST;
> +
> +		if (gup_fd < 0) {
> +			ksft_test_result_skip("gup_test not available\n");
> +			break;
> +		}
> +
> +		if (rw && shared && fs_is_unknown(fs_type)) {
> +			ksft_test_result_skip("Unknown filesystem\n");
> +			return;
> +		}
> +		/*
> +		 * R/O pinning or pinning in a private mapping is always
> +		 * expected to work. Otherwise, we expect long-term R/W pinning
> +		 * to only succeed for special fielesystems.
> +		 */
> +		should_work = !shared || !rw ||
> +			      fs_supports_writable_longterm_pinning(fs_type);
> +
> +		args.addr = (__u64)(uintptr_t)mem;
> +		args.size = size;
> +		args.flags = fast ? PIN_LONGTERM_TEST_FLAG_USE_FAST : 0;
> +		args.flags |= rw ? PIN_LONGTERM_TEST_FLAG_USE_WRITE : 0;
> +		ret = ioctl(gup_fd, PIN_LONGTERM_TEST_START, &args);
> +		if (ret && errno == EINVAL) {
> +			ksft_test_result_skip("PIN_LONGTERM_TEST_START failed\n");
> +			break;
> +		} else if (ret && errno == EFAULT) {
> +			ksft_test_result(!should_work, "Should have failed\n");
> +			break;
> +		} else if (ret) {
> +			ksft_test_result_fail("PIN_LONGTERM_TEST_START failed\n");
> +			break;
> +		}
> +
> +		if (ioctl(gup_fd, PIN_LONGTERM_TEST_STOP))
> +			ksft_print_msg("[INFO] PIN_LONGTERM_TEST_STOP failed\n");
> +
> +		/*
> +		 * TODO: if the kernel ever supports long-term R/W pinning on
> +		 * some previously unsupported filesystems, we might want to
> +		 * perform some additional tests for possible data corruptions.
> +		 */
> +		ksft_test_result(should_work, "Should have worked\n");
> +		break;
> +	}
> +	default:
> +		assert(false);
> +	}
> +
> +	munmap(mem, size);
> +}
> +
> +typedef void (*test_fn)(int fd, size_t size);
> +
> +static void run_with_memfd(test_fn fn, const char *desc)
> +{
> +	int fd;
> +
> +	ksft_print_msg("[RUN] %s ... with memfd\n", desc);
> +
> +	fd = memfd_create("test", 0);
> +	if (fd < 0) {
> +		ksft_test_result_fail("memfd_create() failed\n");
> +		return;
> +	}
> +
> +	fn(fd, pagesize);
> +	close(fd);
> +}
> +
> +static void run_with_tmpfile(test_fn fn, const char *desc)
> +{
> +	FILE *file;
> +	int fd;
> +
> +	ksft_print_msg("[RUN] %s ... with tmpfile\n", desc);
> +
> +	file = tmpfile();
> +	if (!file) {
> +		ksft_test_result_fail("tmpfile() failed\n");
> +		return;
> +	}
> +
> +	fd = fileno(file);
> +	if (fd < 0) {
> +		ksft_test_result_fail("fileno() failed\n");
> +		return;
> +	}
> +
> +	fn(fd, pagesize);
> +	fclose(file);
> +}
> +
> +static void run_with_local_tmpfile(test_fn fn, const char *desc)
> +{
> +	char filename[] = __FILE__"_tmpfile_XXXXXX";
> +	int fd;
> +
> +	ksft_print_msg("[RUN] %s ... with local tmpfile\n", desc);
> +
> +	fd = mkstemp(filename);
> +	if (fd < 0) {
> +		ksft_test_result_fail("mkstemp() failed\n");
> +		return;
> +	}
> +
> +	if (unlink(filename)) {
> +		ksft_test_result_fail("unlink() failed\n");
> +		goto close;
> +	}
> +
> +	fn(fd, pagesize);
> +close:
> +	close(fd);
> +}
> +
> +static void run_with_memfd_hugetlb(test_fn fn, const char *desc,
> +				   size_t hugetlbsize)
> +{
> +	int flags = MFD_HUGETLB;
> +	int fd;
> +
> +	ksft_print_msg("[RUN] %s ... with memfd hugetlb (%zu kB)\n", desc,
> +		       hugetlbsize / 1024);
> +
> +	flags |= __builtin_ctzll(hugetlbsize) << MFD_HUGE_SHIFT;

Hm this feels a little cute :)

> +
> +	fd = memfd_create("test", flags);
> +	if (fd < 0) {
> +		ksft_test_result_skip("memfd_create() failed\n");
> +		return;
> +	}
> +
> +	fn(fd, hugetlbsize);
> +	close(fd);
> +}
> +
> +struct test_case {
> +	const char *desc;
> +	test_fn fn;
> +};
> +
> +static void test_shared_rw_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RW, true);
> +}
> +
> +static void test_shared_rw_fast_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RW_FAST, true);
> +}
> +
> +static void test_shared_ro_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RO, true);
> +}
> +
> +static void test_shared_ro_fast_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RO_FAST, true);
> +}
> +
> +static void test_private_rw_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RW, false);
> +}
> +
> +static void test_private_rw_fast_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RW_FAST, false);
> +}
> +
> +static void test_private_ro_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RO, false);
> +}
> +
> +static void test_private_ro_fast_pin(int fd, size_t size)
> +{
> +	do_test(fd, size, TEST_TYPE_RO_FAST, false);
> +}
> +
> +static const struct test_case test_cases[] = {
> +	{
> +		"R/W longterm GUP pin in MAP_SHARED file mapping",
> +		test_shared_rw_pin,
> +	},
> +	{
> +		"R/W longterm GUP-fast pin in MAP_SHARED file mapping",
> +		test_shared_rw_fast_pin,
> +	},
> +	{
> +		"R/O longterm GUP pin in MAP_SHARED file mapping",
> +		test_shared_ro_pin,
> +	},
> +	{
> +		"R/O longterm GUP-fast pin in MAP_SHARED file mapping",
> +		test_shared_ro_fast_pin,
> +	},
> +	{
> +		"R/W longterm GUP pin in MAP_PRIVATE file mapping",
> +		test_private_rw_pin,
> +	},
> +	{
> +		"R/W longterm GUP-fast pin in MAP_PRIVATE file mapping",
> +		test_private_rw_fast_pin,
> +	},
> +	{
> +		"R/O longterm GUP pin in MAP_PRIVATE file mapping",
> +		test_private_ro_pin,
> +	},
> +	{
> +		"R/O longterm GUP-fast pin in MAP_PRIVATE file mapping",
> +		test_private_ro_fast_pin,
> +	},
> +};
> +
> +static void run_test_case(struct test_case const *test_case)
> +{
> +	int i;
> +
> +	run_with_memfd(test_case->fn, test_case->desc);
> +	run_with_tmpfile(test_case->fn, test_case->desc);
> +	run_with_local_tmpfile(test_case->fn, test_case->desc);
> +	for (i = 0; i < nr_hugetlbsizes; i++)
> +		run_with_memfd_hugetlb(test_case->fn, test_case->desc,
> +				       hugetlbsizes[i]);
> +}
> +
> +static int tests_per_test_case(void)
> +{
> +	return 3 + nr_hugetlbsizes;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int i, err;
> +
> +	pagesize = getpagesize();
> +	nr_hugetlbsizes = detect_hugetlb_page_sizes(hugetlbsizes,
> +						    ARRAY_SIZE(hugetlbsizes));
> +
> +	ksft_print_header();
> +	ksft_set_plan(ARRAY_SIZE(test_cases) * tests_per_test_case());
> +
> +	gup_fd = open("/sys/kernel/debug/gup_test", O_RDWR);
> +
> +	for (i = 0; i < ARRAY_SIZE(test_cases); i++)
> +		run_test_case(&test_cases[i]);
> +
> +	err = ksft_get_fail_cnt();
> +	if (err)
> +		ksft_exit_fail_msg("%d out of %d tests failed\n",
> +				   err, ksft_test_num());
> +	return ksft_exit_pass();
> +}
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index 4893eb60d96d..b6b1eb6a8a6b 100644
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -24,7 +24,7 @@ separated by spaces:
>  - mmap
>  	tests for mmap(2)
>  - gup_test
> -	tests for gup using gup_test interface
> +	tests for gup

Super nitty again, but I'm guessing this means the CONFIG_GUP_TEST
interface, perhaps worth keeping?

>  - userfaultfd
>  	tests for  userfaultfd(2)
>  - compaction
> @@ -196,6 +196,8 @@ CATEGORY="gup_test" run_test ./gup_test -a
>  # Dump pages 0, 19, and 4096, using pin_user_pages:
>  CATEGORY="gup_test" run_test ./gup_test -ct -F 0x1 0 19 0x1000
>
> +CATEGORY="gup_test" run_test ./gup_longterm
> +
>  CATEGORY="userfaultfd" run_test ./uffd-unit-tests
>  uffd_stress_bin=./uffd-stress
>  CATEGORY="userfaultfd" run_test ${uffd_stress_bin} anon 20 16
> --
> 2.40.1
>

OK this patch is really nice + well implemented, I can only point out a
couple EXTREMELY nitty comments :) Thanks very much for adding a test for
this, it's super useful!

Therefore,

Reviewed-by: Lorenzo Stoakes <lstoakes@...il.com>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ