netdev - Re: [PATCH net-next v5 3/3] connector/cn

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]

Message-ID: <20241018100440.GF1697@kernel.org>
Date: Fri, 18 Oct 2024 11:04:40 +0100
From: Simon Horman <horms@...nel.org>
To: Anjali Kulkarni <anjali.k.kulkarni@...cle.com>
Cc: davem@...emloft.net, Liam.Howlett@...cle.com, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, mingo@...hat.com,
	peterz@...radead.org, juri.lelli@...hat.com,
	vincent.guittot@...aro.org, dietmar.eggemann@....com,
	rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
	vschneid@...hat.com, jiri@...nulli.us, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, akpm@...ux-foundation.org, shuah@...nel.org,
	linux-kselftest@...r.kernel.org, peili.io@...cle.com
Subject: Re: [PATCH net-next v5 3/3] connector/cn_proc: Selftest for threads

On Thu, Oct 17, 2024 at 11:14:36AM -0700, Anjali Kulkarni wrote:
> Test to check if setting PROC_CN_MCAST_NOTIFY in proc connector API, allows
> a thread's non-zero exit status to be returned to proc_filter.
> 
> The threads.c program creates 2 child threads. 1st thread handles signal
> SIGSEGV, and 2nd thread needs to indicate some error condition (value 1)
> to the kernel, instead of using pthread_exit() with 1.
> 
> In both cases, child sends notify_netlink_thread_exit(exit_code) to kernel,
> to let kernel know it has exited abnormally with exit_code.
> 
> Compile:
>     make thread
>     make proc_filter
> Run:
>     ./threads
> 
> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@...cle.com>
> ---
>  tools/testing/selftests/connector/Makefile    |  23 +-
>  .../testing/selftests/connector/proc_filter.c |  34 ++-
>  tools/testing/selftests/connector/thread.c    | 232 ++++++++++++++++++
>  .../selftests/connector/thread_filter.c       |  96 ++++++++
>  4 files changed, 378 insertions(+), 7 deletions(-)
>  create mode 100644 tools/testing/selftests/connector/thread.c
>  create mode 100644 tools/testing/selftests/connector/thread_filter.c
> 
> diff --git a/tools/testing/selftests/connector/Makefile b/tools/testing/selftests/connector/Makefile
> index 92188b9bac5c..bf335826bc3b 100644
> --- a/tools/testing/selftests/connector/Makefile
> +++ b/tools/testing/selftests/connector/Makefile
> @@ -1,5 +1,26 @@
>  # SPDX-License-Identifier: GPL-2.0
> -CFLAGS += -Wall $(KHDR_INCLUDES)
> +KERNEL="../../../.."
> +
> +CFLAGS += -Wall $(KHDR_INCLUDES) -I $(KERNEL)/include/uapi -I $(KERNEL)/include
> +
> +proc_filter: proc_filter.o
> +	cc proc_filter.o -o proc_filter
> +
> +proc_filter.o: proc_filter.c
> +	cc -c proc_filter.c -o proc_filter.o $(CFLAGS)
> +
> +thread: thread.o thread_filter.o
> +	cc thread.o thread_filter.o -o thread
> +
> +thread.o: thread.c $(DEPS)
> +		cc -c thread.c -o thread.o $(CFLAGS)
> +
> +thread_filter.o: thread_filter.c
> +		cc -c thread_filter.c -o thread_filter.o $(CFLAGS)
> +
> +define EXTRA_CLEAN
> +	rm *.o thread
> +endef
>  
>  TEST_GEN_PROGS = proc_filter
>  

I am a little confused by this, as it seems to result in user-space
code using kernel headers. Is that expected?

$ make -C tools/testing/selftests/connector
...
cc -c proc_filter.c -o proc_filter.o -Wall -isystem /home/horms/projects/linux/linux/tools/testing/selftests/../../../usr/include -I "../../../.."/include/uapi -I "../../../.."/include -D_GNU_SOURCE=
In file included from ../../../../include/uapi/linux/netlink.h:7,
                 from proc_filter.c:11:
../../../../include/uapi/linux/types.h:10:2: warning: #warning "Attempt to use kernel headers from user space, see https://kernelnewbies.org/KernelHeaders" [-Wcpp]
   10 | #warning "Attempt to use kernel headers from user space, see https://kernelnewbies.org/KernelHeaders"
      |  ^~~~~~~
...

> diff --git a/tools/testing/selftests/connector/thread.c b/tools/testing/selftests/connector/thread.c

...

> +static inline void init_threads(pthread_attr_t *attr)

Please don't use inline in .c files unless there is a demonstrable,
usually performance, reason to do so.

Likewise twice more in this patch and once in patch 1/3.

> +{
> +	int ret;
> +
> +	ret = pthread_attr_init(attr);
> +	if (ret != 0) {
> +		perror("pthread_attr_init failed");
> +		exit(ret);
> +	}
> +
> +	ret = pthread_attr_setdetachstate(attr, PTHREAD_CREATE_DETACHED);
> +	if (ret != 0) {
> +		perror("pthread_attr_setdetachstate failed");
> +		exit(ret);
> +	}
> +}

...