Re: [PATCH v3 09/13] selftests/mm: skip uffd-wp-mremap if UFFD write-protect is unsupported

[Sayali Patil]

On 27/03/26 12:46, Sayali Patil wrote:
> The uffd-wp-mremap test requires the UFFD_FEATURE_PAGEFAULT_FLAG_WP
> capability. On systems where userfaultfd write-protect is
> not supported, uffd_register() fails and the test reports failures.
>
> Check for the required feature at startup and skip the test when the
> UFFD_FEATURE_PAGEFAULT_FLAG_WP capability is not present,
> preventing false failures on unsupported configurations.
>
> Before patch:
>   running ./uffd-wp-mremap
>   ------------------------
>    [INFO] detected THP size: 256 KiB
>    [INFO] detected THP size: 512 KiB
>    [INFO] detected THP size: 1024 KiB
>    [INFO] detected THP size: 2048 KiB
>    [INFO] detected hugetlb page size: 2048 KiB
>    [INFO] detected hugetlb page size: 1048576 KiB
>   1..24
>    [RUN] test_one_folio(size=65536, private=false, swapout=false,
>    hugetlb=false)
>   not ok 1 uffd_register() failed
>    [RUN] test_one_folio(size=65536, private=true, swapout=false,
>    hugetlb=false)
>   not ok 2 uffd_register() failed
>    [RUN] test_one_folio(size=65536, private=false, swapout=true,
>    hugetlb=false)
>   not ok 3 uffd_register() failed
>    [RUN] test_one_folio(size=65536, private=true, swapout=true,
>    hugetlb=false)
>   not ok 4 uffd_register() failed
>    [RUN] test_one_folio(size=262144, private=false, swapout=false,
>    hugetlb=false)
>   not ok 5 uffd_register() failed
>    [RUN] test_one_folio(size=524288, private=false, swapout=false,
>    hugetlb=false)
>   not ok 6 uffd_register() failed
>   .
>   .
>   .
>   Bail out! 24 out of 24 tests failed
>    Totals: pass:0 fail:24 xfail:0 xpass:0 skip:0 error:0
>   [FAIL]
> not ok 1 uffd-wp-mremap # exit=1
>
> After patch:
>   running ./uffd-wp-mremap
>   ------------------------
>   1..0 # SKIP uffd-wp feature not supported
>   [SKIP]
> ok 1 uffd-wp-mremap # SKIP
>
> Acked-by: Zi Yan <ziy@xxxxxxxxxx>
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> Signed-off-by: Sayali Patil <sayalip@xxxxxxxxxxxxx>
> ---
>   tools/testing/selftests/mm/uffd-wp-mremap.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/tools/testing/selftests/mm/uffd-wp-mremap.c b/tools/testing/selftests/mm/uffd-wp-mremap.c
> index 17186d4a4147..6edbd09f0ca6 100644
> --- a/tools/testing/selftests/mm/uffd-wp-mremap.c
> +++ b/tools/testing/selftests/mm/uffd-wp-mremap.c
> @@ -19,6 +19,17 @@ static size_t thpsizes[20];
>   static int nr_hugetlbsizes;
>   static size_t hugetlbsizes[10];
>   
> +static void check_uffd_wp_feature_supported(void)
> +{
> +	uint64_t features;
> +
> +	if (uffd_get_features(&features) && errno == ENOENT)
> +		ksft_exit_skip("failed to get available features (%d)\n", errno);
> +
> +	if (!(features & UFFD_FEATURE_PAGEFAULT_FLAG_WP))
> +		ksft_exit_skip("uffd-wp feature not supported\n");
> +}
> +
>   static int detect_thp_sizes(size_t sizes[], int max)
>   {
>   	int count = 0;
> @@ -336,6 +347,8 @@ int main(int argc, char **argv)
>   	struct thp_settings settings;
>   	int i, j, plan = 0;
>   
> +	check_uffd_wp_feature_supported();
> +
>   	pagesize = getpagesize();
>   	nr_thpsizes = detect_thp_sizes(thpsizes, ARRAY_SIZE(thpsizes));
>   	nr_hugetlbsizes = detect_hugetlb_page_sizes(hugetlbsizes,

AI review question:

> commit 22914640356d1eaf4e0a8a8b13ad71911d12db50
> Author: Sayali Patil <sayalip@xxxxxxxxxxxxx>
> selftests/mm: skip uffd-wp-mremap if UFFD write-protect is unsupported
> This commit adds a check at startup to verify if the userfaultfd
> write-protect capability is present. It skips the test instead of reporting> false failures on unsupported configurations.
> > diff --git a/tools/testing/selftests/mm/uffd-wp-mremap.c b/tools/testing/selftests/mm/uffd-wp-mremap.c> > index 17186d4a4147..6edbd09f0ca6 100644
> > --- a/tools/testing/selftests/mm/uffd-wp-mremap.c
> > +++ b/tools/testing/selftests/mm/uffd-wp-mremap.c
> > @@ -19,6 +19,17 @@ static size_t thpsizes[20];
> > static int nr_hugetlbsizes;
> > static size_t hugetlbsizes[10];
>
> > +static void check_uffd_wp_feature_supported(void)
> > +{
> > +	uint64_t features;
> > +
> > +	if (uffd_get_features(&features) && errno == ENOENT)
> > +		ksft_exit_skip("failed to get available features (%d)\n", errno);
> If uffd_get_features() fails but errno is something other than ENOENT (for> example, EPERM when unprivileged userfaultfd is disabled), this check will
> evaluate to false and execution will continue.> > +
> > +	if (!(features & UFFD_FEATURE_PAGEFAULT_FLAG_WP))
> Could this evaluate uninitialized stack memory?
> If uffd_get_features() fails and is bypassed by the previous check, the
> features variable will remain uninitialized. This might cause the test to
> falsely pass or fail depending on whatever data happens to be on the stack.> Should we initialize features to zero or exit on all uffd_get_features()
> failures instead?
> > +		ksft_exit_skip("uffd-wp feature not supported\n");
> > +}

Yes,If uffd_get_features() fails with an error other than ENOENT, the 
features variable would remain uninitialized, which could lead to 
undefined behavior in the subsequent check.

I will fix this in the next patch version.

Thanks,
Sayali