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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The uffd selftests unconditionally call `err()` if `madvise(MADV_NOHUGEPAGE)` fails for RWP tests, which will abort the entire test suite on kernels compiled with `CONFIG_TRANSPARENT_HUGEPAGE=n`.
- [Medium] The `rwp-fork-pin` and `rwp-mprotect` tests will silently pass if a kernel bug causes the `uffd-wp` marker to be completely dropped instead of preserved.
--

commit c7d4aef17412fdd00834c4bcc5b4fbabc6db2f28
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>
Date:   Mon Jul 6 08:48:03 2026 +0000

    selftests/mm: add userfaultfd RWP tests
    
    Coverage for UFFDIO_REGISTER_MODE_RWP and UFFDIO_RWPROTECT. All tests
    run against anon, shmem, shmem-private, hugetlb, and hugetlb-private
    memory, except rwp-fork-pin which is anon-only.

> 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

[ ... ]

> +static void uffd_rwp_mprotect_test(uffd_global_test_opts_t *gopts,
> +				   uffd_test_args_t *args)
> +{

[ ... ]

> +	/* Round-trip mprotect(): PROT_NONE -> PROT_READ|PROT_WRITE */
> +	if (mprotect(gopts->area_dst, nr_pages * page_size, PROT_NONE))
> +		err("mprotect() PROT_NONE");
> +	if (mprotect(gopts->area_dst, nr_pages * page_size,
> +		     PROT_READ | PROT_WRITE))
> +		err("mprotect() PROT_READ|PROT_WRITE");
> +
> +	/* Touch every page. Async RWP must auto-resolve each fault. */
> +	for (p = 0; p < nr_pages; p++) {
> +		volatile char *page = gopts->area_dst + p * page_size;
> +		(void)*page;
> +	}

[Severity: Medium]
Will this test falsely pass if mprotect() completely drops the uffd-wp
marker? Since the test accesses the pages expecting async faults to clear the
markers, but doesn't assert that the markers survived the mprotect()
round-trip first, a kernel bug dropping the protection might let the access
silently succeed and leave the marker clear.

[ ... ]

> +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]
Could this test also falsely pass if copy_present_page() drops the uffd-wp
marker completely? The child touches the page and verifies the marker is
clear, but it doesn't verify the marker was correctly preserved prior to the
access.

[ ... ]

> @@ -1814,6 +2684,20 @@ int main(int argc, char *argv[])
>  				uffd_test_skip(errmsg);
>  				continue;
>  			}
> +			/*
> +			 * RWP tracks protection on ptes; a THP-backed shmem/anon
> +			 * range (e.g. shmem_enabled=always) would split on
> +			 * rwprotect and change behaviour under the test. Keep
> +			 * such ranges off THP. hugetlb is huge by definition and
> +			 * rejects MADV_NOHUGEPAGE, so skip it.
> +			 */
> +			if ((test->uffd_feature_required & UFFD_FEATURE_RWP) &&
> +			    !(mem_type->mem_flag & (MEM_HUGETLB | MEM_HUGETLB_PRIVATE))) {
> +				unsigned long len = gopts.nr_pages * gopts.page_size;
> +
> +				if (madvise(gopts.area_dst, len, MADV_NOHUGEPAGE))
> +					err("madvise(MADV_NOHUGEPAGE)");

[Severity: Medium]
Will this unconditional error check on madvise() abort the entire test suite
on kernels lacking THP support? On a kernel compiled with
CONFIG_TRANSPARENT_HUGEPAGE=n, madvise(MADV_NOHUGEPAGE) returns -EINVAL,
which would cause err() to exit the process instead of gracefully continuing
or skipping the test.

> +			}
>  			test->uffd_fn(&gopts, &args);
>  			uffd_test_ctx_clear(&gopts);
>  		}

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