linux-kernel - Re: [PATCH v2 16/31] selftests/mm: UFFDIO

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]
Date:   Wed, 12 Apr 2023 12:47:39 -0700
From:   Axel Rasmussen <axelrasmussen@...gle.com>
To:     Peter Xu <peterx@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Nadav Amit <nadav.amit@...il.com>,
        David Hildenbrand <david@...hat.com>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Leonardo Bras Soares Passos <lsoaresp@...hat.com>
Subject: Re: [PATCH v2 16/31] selftests/mm: UFFDIO_API test

On Wed, Apr 12, 2023 at 9:43 AM Peter Xu <peterx@...hat.com> wrote:
>
> Add one simple test for UFFDIO_API.  With that, I also added a bunch of
> small but handy helpers along the way.
>
> Signed-off-by: Peter Xu <peterx@...hat.com>
> ---
>  tools/testing/selftests/mm/uffd-unit-tests.c | 109 ++++++++++++++++++-
>  1 file changed, 108 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index 6857388783be..bb492c258486 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -9,9 +9,116 @@
>
>  #ifdef __NR_userfaultfd
>
> +static void uffd_test_report(void)
> +{
> +       printf("Userfaults unit tests: pass=%u, skip=%u, fail=%u (total=%u)\n",
> +              ksft_get_pass_cnt(),
> +              ksft_get_xskip_cnt(),
> +              ksft_get_fail_cnt(),
> +              ksft_test_num());
> +}
> +
> +static void uffd_test_pass(void)
> +{
> +       printf("done\n");
> +       ksft_inc_pass_cnt();
> +}
> +
> +#define  uffd_test_start(...)  do {            \
> +               printf("Testing ");             \
> +               printf(__VA_ARGS__);            \
> +               printf("... ");                 \
> +               fflush(stdout);                 \
> +       } while (0)
> +
> +#define  uffd_test_fail(...)  do {             \
> +               printf("failed [reason: ");     \
> +               printf(__VA_ARGS__);            \
> +               printf("]\n");                  \
> +               ksft_inc_fail_cnt();            \
> +       } while (0)
> +
> +#define  uffd_test_skip(...)  do {             \
> +               printf("skipped [reason: ");    \
> +               printf(__VA_ARGS__);            \
> +               printf("]\n");                  \
> +               ksft_inc_xskip_cnt();           \
> +       } while (0)

Some of this is duplicating what's in kselftest_harness.h - e.g.
assertions, printing test summary, keeping track of pass/fail/skip
counts, etc.

I wonder how you feel about using the standard harness? E.g. is the
plan to do this (more straightforward?) refactor first, and then
switch later? Or is there some reason it can't be used?

> +
> +/*
> + * Returns 1 if specific userfaultfd supported, 0 otherwise.  Note, we'll
> + * return 1 even if some test failed as long as uffd supported, because in
> + * that case we still want to proceed with the rest uffd unit tests.
> + */
> +static int test_uffd_api(bool use_dev)
> +{
> +       struct uffdio_api uffdio_api;
> +       int uffd;
> +
> +       uffd_test_start("UFFDIO_API (with %s)",
> +                       use_dev ? "/dev/userfaultfd" : "syscall");
> +
> +       if (use_dev)
> +               uffd = uffd_open_dev(UFFD_FLAGS);
> +       else
> +               uffd = uffd_open_sys(UFFD_FLAGS);
> +       if (uffd < 0) {
> +               uffd_test_skip("cannot open userfaultfd handle");
> +               return 0;
> +       }
> +
> +       /* Test wrong UFFD_API */
> +       uffdio_api.api = 0xab;
> +       uffdio_api.features = 0;
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api) == 0) {
> +               uffd_test_fail("UFFDIO_API should fail with wrong api but didn't");
> +               goto out;
> +       }
> +
> +       /* Test wrong feature bit */
> +       uffdio_api.api = UFFD_API;
> +       uffdio_api.features = BIT_ULL(63);
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api) == 0) {
> +               uffd_test_fail("UFFDIO_API should fail with wrong feature but didn't");
> +               goto out;
> +       }
> +
> +       /* Test normal UFFDIO_API */
> +       uffdio_api.api = UFFD_API;
> +       uffdio_api.features = 0;
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api)) {
> +               uffd_test_fail("UFFDIO_API should succeed but failed");
> +               goto out;
> +       }
> +
> +       /* Test double requests of UFFDIO_API with a random feature set */
> +       uffdio_api.features = BIT_ULL(0);
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api) == 0) {
> +               uffd_test_fail("UFFDIO_API should reject initialized uffd");
> +               goto out;
> +       }
> +
> +       uffd_test_pass();
> +out:
> +       close(uffd);
> +       /* We have a valid uffd handle */
> +       return 1;
> +}
> +
>  int main(int argc, char *argv[])
>  {
> -       return KSFT_PASS;
> +       int has_uffd;
> +
> +       has_uffd = test_uffd_api(false);
> +       has_uffd |= test_uffd_api(true);
> +
> +       if (!has_uffd) {
> +               printf("Userfaultfd not supported or unprivileged, skip all tests\n");
> +               exit(KSFT_SKIP);
> +       }
> +       uffd_test_report();
> +
> +       return ksft_get_fail_cnt() ? KSFT_FAIL : KSFT_PASS;
>  }
>
>  #else /* __NR_userfaultfd */
> --
> 2.39.1
>