linux-kernel - Re: [PATCH] selftests/mm: Reduce uffd-unit-test poison test to minimum

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJHvVchZHQCQnO48Q3OhTPYncZdXSoBc1CK-CHz_XAOO+CL9gA@mail.gmail.com>
Date: Thu, 26 Jun 2025 10:32:20 -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>, Ujwal Kundur <ujwal.kundur@...il.com>, 
	David Hildenbrand <david@...hat.com>, shuah@...nel.org, jackmanb@...gle.com
Subject: Re: [PATCH] selftests/mm: Reduce uffd-unit-test poison test to minimum

On Fri, Jun 20, 2025 at 8:01 AM Peter Xu <peterx@...hat.com> wrote:
>
> The test will still generate quite some unwanted MCE error messages to
> syslog.  There was old proposal ratelimiting the MCE messages from kernel,
> but that has risk of hiding real useful information on production systems.
>
> We can at least reduce the test to minimum to not over-pollute dmesg,
> however trying to not lose its coverage too much.
>
> Cc: Axel Rasmussen <axelrasmussen@...gle.com>
> Signed-off-by: Peter Xu <peterx@...hat.com>

Besides a small nitpick you can take:

Reviewed-by: Axel Rasmussen <axelrasmussen@...gle.com>

Making the functional tests small makes sense to me, especially for
poisoning. Only reason to use a huge number of pages is if we're
trying to stress racy bugs or so, but really for that you'd want even
more pages / more threads / run for a longer time. It makes sense to
separate that use case out / maybe not run it by default, and leave
the functional tests small + fast.

> ---
>  tools/testing/selftests/mm/uffd-unit-tests.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index c73fd5d455c8..39b3fd1b7bf2 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -1027,6 +1027,9 @@ static void uffd_poison_handle_fault(
>                 do_uffdio_poison(uffd, offset);
>  }
>
> +/* Make sure to cover odd/even, and minimum duplications */
> +#define  UFFD_POISON_TEST_NPAGES  4
> +
>  static void uffd_poison_test(uffd_test_args_t *targs)
>  {
>         pthread_t uffd_mon;
> @@ -1034,12 +1037,17 @@ static void uffd_poison_test(uffd_test_args_t *targs)
>         struct uffd_args args = { 0 };
>         struct sigaction act = { 0 };
>         unsigned long nr_sigbus = 0;
> -       unsigned long nr;
> +       unsigned long nr, poison_pages = UFFD_POISON_TEST_NPAGES;
> +
> +       if (nr_pages < poison_pages) {
> +               uffd_test_skip("Too less pages for POISON test");

I think "Too few pages for POISON test" is more grammatically correct.

> +               return;
> +       }
>
>         fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
>
> -       uffd_register_poison(uffd, area_dst, nr_pages * page_size);
> -       memset(area_src, 0, nr_pages * page_size);
> +       uffd_register_poison(uffd, area_dst, poison_pages * page_size);
> +       memset(area_src, 0, poison_pages * page_size);
>
>         args.handle_fault = uffd_poison_handle_fault;
>         if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
> @@ -1051,7 +1059,7 @@ static void uffd_poison_test(uffd_test_args_t *targs)
>         if (sigaction(SIGBUS, &act, 0))
>                 err("sigaction");
>
> -       for (nr = 0; nr < nr_pages; ++nr) {
> +       for (nr = 0; nr < poison_pages; ++nr) {
>                 unsigned long offset = nr * page_size;
>                 const char *bytes = (const char *) area_dst + offset;
>                 const char *i;
> @@ -1078,9 +1086,9 @@ static void uffd_poison_test(uffd_test_args_t *targs)
>         if (pthread_join(uffd_mon, NULL))
>                 err("pthread_join()");
>
> -       if (nr_sigbus != nr_pages / 2)
> +       if (nr_sigbus != poison_pages / 2)
>                 err("expected to receive %lu SIGBUS, actually received %lu",
> -                   nr_pages / 2, nr_sigbus);
> +                   poison_pages / 2, nr_sigbus);
>
>         uffd_test_pass();
>  }
> --
> 2.49.0
>