Re: [PATCH v8 14/15] selftests/mm: add userfaultfd RWP tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `uffd_rwp_fork_pin_test` will falsely pass if `copy_present_page()` drops both the `PAGE_NONE` protection and the `uffd-wp` marker during fork.
--

commit e0500399155340fb34ee7e3769e6b868bb0068f6
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

selftests/mm: add userfaultfd RWP tests

This commit introduces several selftests for UFFDIO_REGISTER_MODE_RWP and
UFFDIO_RWPROTECT modes. It includes tests for async/sync modes, pagemap
scanning, mprotect, fork behavior, and swap out/in interactions across
different memory types.

> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index a6c14109e8188..bd3b9f41ee354 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
[ ... ]
> @@ -602,6 +617,784 @@ void uffd_minor_collapse_test(uffd_global_test_opts_t *gopts, uffd_test_args_t *
[ ... ]
> +static void uffd_rwp_fork_pin_test(uffd_global_test_opts_t *gopts,
> +				   uffd_test_args_t *args)
> +{
[ ... ]
> +	child = fork();
> +	if (child < 0)
> +		err("fork");
> +	if (child == 0) {
> +		volatile char c;
> +		int cfd;
> +
> +		/*
> +		 * Read the pinned page. Only reaches the fault path if the
> +		 * child PTE is protnone + uffd; async mode auto-resolves and
> +		 * clears the uffd bit. If copy_present_page() dropped
> +		 * PAGE_NONE, the read would silently succeed and the bit
> +		 * would still be set.
> +		 */
> +		c = *(volatile char *)gopts->area_dst;
> +		(void)c;
> +
> +		cfd = pagemap_open();
> +		value = pagemap_get_entry(cfd, gopts->area_dst);
> +		close(cfd);
> +		_exit((value & PM_UFFD_WP) ? 1 : 0);
> +	}

[Severity: Medium]
Will this test falsely pass if copy_present_page() drops both PAGE_NONE and
the uffd-wp marker during fork()?

If both the protection and the marker are erroneously dropped, the child's read
will succeed silently without a fault, and the subsequent pagemap check will
find the uffd-wp bit cleared (since it was never inherited). The child would
then exit with 0, falsely reporting a PASS.

Should there be a precondition check to assert that the PM_UFFD_WP marker
actually exists in the child's pagemap before performing the read?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703133615.1039465-1-kirill@shutemov.name?part=14