netdev - Re: [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread

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] [day] [month] [year] [list]
Message-ID: <c54bae70-3655-4188-aad7-358bb7897c3c@collabora.com>
Date: Sat, 30 Mar 2024 00:54:20 +0500
From: Muhammad Usama Anjum <usama.anjum@...labora.com>
To: Edward Liaw <edliaw@...gle.com>, linux-kernel@...r.kernel.org,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Shuah Khan <shuah@...nel.org>,
 Nathan Chancellor <nathan@...nel.org>,
 Nick Desaulniers <ndesaulniers@...gle.com>, Bill Wendling
 <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 Jann Horn <jannh@...gle.com>
Cc: Muhammad Usama Anjum <usama.anjum@...labora.com>,
 linux-kselftest@...r.kernel.org, kernel-team@...roid.com,
 Lokesh Gidra <lokeshgidra@...gle.com>, bpf@...r.kernel.org,
 netdev@...r.kernel.org, linux-mm@...ck.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v3] selftests/mm: Fix ARM related issue with fork after
 pthread_create

On 3/26/24 12:40 AM, Edward Liaw wrote:
> Following issue was observed while running the uffd-unit-tests selftest
> on ARM devices. On x86_64 no issues were detected:
> 
> pthread_create followed by fork caused deadlock in certain cases
> wherein fork required some work to be completed by the created thread.
> Used synchronization to ensure that created thread's start function has
> started before invoking fork.
> 
> Signed-off-by: Lokesh Gidra <lokeshgidra@...gle.com>
> [edliaw: Refactored to use atomic_bool]
> Signed-off-by: Edward Liaw <edliaw@...gle.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@...labora.com>

> ---
> 
> v2: restored accidentally removed uffd_test_case_ops when merging
> v3: fixed commit subject to use selftests/mm prefix
> 
>  tools/testing/selftests/mm/uffd-common.c     |  3 +++
>  tools/testing/selftests/mm/uffd-common.h     |  2 ++
>  tools/testing/selftests/mm/uffd-unit-tests.c | 10 ++++++++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c
> index b0ac0ec2356d..7ad6ba660c7d 100644
> --- a/tools/testing/selftests/mm/uffd-common.c
> +++ b/tools/testing/selftests/mm/uffd-common.c
> @@ -18,6 +18,7 @@ bool test_uffdio_wp = true;
>  unsigned long long *count_verify;
>  uffd_test_ops_t *uffd_test_ops;
>  uffd_test_case_ops_t *uffd_test_case_ops;
> +atomic_bool ready_for_fork;
> 
>  static int uffd_mem_fd_create(off_t mem_size, bool hugetlb)
>  {
> @@ -518,6 +519,8 @@ void *uffd_poll_thread(void *arg)
>  	pollfd[1].fd = pipefd[cpu*2];
>  	pollfd[1].events = POLLIN;
> 
> +	ready_for_fork = true;
> +
>  	for (;;) {
>  		ret = poll(pollfd, 2, -1);
>  		if (ret <= 0) {
> diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
> index cb055282c89c..cc5629c3d2aa 100644
> --- a/tools/testing/selftests/mm/uffd-common.h
> +++ b/tools/testing/selftests/mm/uffd-common.h
> @@ -32,6 +32,7 @@
>  #include <inttypes.h>
>  #include <stdint.h>
>  #include <sys/random.h>
> +#include <stdatomic.h>
> 
>  #include "../kselftest.h"
>  #include "vm_util.h"
> @@ -103,6 +104,7 @@ extern bool map_shared;
>  extern bool test_uffdio_wp;
>  extern unsigned long long *count_verify;
>  extern volatile bool test_uffdio_copy_eexist;
> +extern atomic_bool ready_for_fork;
> 
>  extern uffd_test_ops_t anon_uffd_test_ops;
>  extern uffd_test_ops_t shmem_uffd_test_ops;
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index 2b9f8cc52639..4a48dc617c6b 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -775,6 +775,8 @@ static void uffd_sigbus_test_common(bool wp)
>  	char c;
>  	struct uffd_args args = { 0 };
> 
> +	ready_for_fork = false;
> +
>  	fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
> 
>  	if (uffd_register(uffd, area_dst, nr_pages * page_size,
> @@ -790,6 +792,9 @@ static void uffd_sigbus_test_common(bool wp)
>  	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
>  		err("uffd_poll_thread create");
> 
> +	while (!ready_for_fork)
> +		; /* Wait for the poll_thread to start executing before forking */
> +
>  	pid = fork();
>  	if (pid < 0)
>  		err("fork");
> @@ -829,6 +834,8 @@ static void uffd_events_test_common(bool wp)
>  	char c;
>  	struct uffd_args args = { 0 };
> 
> +	ready_for_fork = false;
> +
>  	fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
>  	if (uffd_register(uffd, area_dst, nr_pages * page_size,
>  			  true, wp, false))
> @@ -838,6 +845,9 @@ static void uffd_events_test_common(bool wp)
>  	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
>  		err("uffd_poll_thread create");
> 
> +	while (!ready_for_fork)
> +		; /* Wait for the poll_thread to start executing before forking */
> +
>  	pid = fork();
>  	if (pid < 0)
>  		err("fork");
> --
> 2.44.0.396.g6e790dbe36-goog
> 
> 

-- 
BR,
Muhammad Usama Anjum