Message-ID: <20230322004752.30055-1-kuniyu@amazon.com>
Date:   Tue, 21 Mar 2023 17:47:52 -0700
From:   Kuniyuki Iwashima <kuniyu@...zon.com>
To:     <aleksandr.mikhalitsyn@...onical.com>
CC:     <arnd@...db.de>, <brauner@...nel.org>, <davem@...emloft.net>,
        <dsahern@...nel.org>, <edumazet@...gle.com>,
        <keescook@...omium.org>, <kuba@...nel.org>, <leon@...nel.org>,
        <linux-arch@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-kselftest@...r.kernel.org>, <netdev@...r.kernel.org>,
        <pabeni@...hat.com>
Subject: Re: [PATCH net-next v2 3/3] selftests: net: add SCM_PIDFD / SO_PEERPIDFD test

From:   Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
Date:   Tue, 21 Mar 2023 19:33:42 +0100
> Basic test to check consistency between:
> - SCM_CREDENTIALS and SCM_PIDFD
> - SO_PEERCRED and SO_PEERPIDFD
> 
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: Leon Romanovsky <leon@...nel.org>
> Cc: David Ahern <dsahern@...nel.org>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Christian Brauner <brauner@...nel.org>
> Cc: linux-kernel@...r.kernel.org
> Cc: netdev@...r.kernel.org
> Cc: linux-arch@...r.kernel.org
> Cc: linux-kselftest@...r.kernel.org
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
> ---
>  tools/testing/selftests/net/.gitignore        |   1 +
>  tools/testing/selftests/net/af_unix/Makefile  |   3 +-
>  .../testing/selftests/net/af_unix/scm_pidfd.c | 336 ++++++++++++++++++
>  3 files changed, 339 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/net/af_unix/scm_pidfd.c
> 
> diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
> index a6911cae368c..f2d23a1df596 100644
> --- a/tools/testing/selftests/net/.gitignore
> +++ b/tools/testing/selftests/net/.gitignore
> @@ -25,6 +25,7 @@ reuseport_bpf_cpu
>  reuseport_bpf_numa
>  reuseport_dualstack
>  rxtimestamp
> +scm_pidfd
>  sk_bind_sendto_listen
>  sk_connect_zero_addr
>  socket
> diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
> index 1e4b397cece6..221c387a7d7f 100644
> --- a/tools/testing/selftests/net/af_unix/Makefile
> +++ b/tools/testing/selftests/net/af_unix/Makefile
> @@ -1,3 +1,4 @@
> -TEST_GEN_PROGS := diag_uid test_unix_oob unix_connect
> +CFLAGS += $(KHDR_INCLUDES)
> +TEST_GEN_PROGS := diag_uid test_unix_oob unix_connect scm_pidfd
>  
>  include ../../lib.mk
> diff --git a/tools/testing/selftests/net/af_unix/scm_pidfd.c b/tools/testing/selftests/net/af_unix/scm_pidfd.c
> new file mode 100644
> index 000000000000..fa502510ee9e
> --- /dev/null
> +++ b/tools/testing/selftests/net/af_unix/scm_pidfd.c
> @@ -0,0 +1,336 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <error.h>
> +#include <limits.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/socket.h>
> +#include <linux/socket.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <sys/un.h>
> +#include <sys/signal.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>

#include "../../kselftest_harness.h"

Could you rewrite with kselftest ?
https://www.kernel.org/doc/html/latest/dev-tools/kselftest.html

Also, it would be better to have all combinations as FIXTURE_VARIANT()
covered by unix_passcred_enabled() like

  (self, peer) = (0, 0), (SO_PASSPIDFD, 0), (0, SO_PASSPIDFD),
                 (SO_PASSPIDFD, SO_PASSPIDFD), ...
                 (SO_PASSPIDFD, SO_PEERCRED), ...

Thanks,
Kuniyuki


> +
> +#define clean_errno() (errno == 0 ? "None" : strerror(errno))
> +#define log_err(MSG, ...)                                                   \
> +	fprintf(stderr, "(%s:%d: errno: %s) " MSG "\n", __FILE__, __LINE__, \
> +		clean_errno(), ##__VA_ARGS__)
> +
> +#ifndef SCM_PIDFD
> +#define SCM_PIDFD 0x04
> +#endif
> +
> +static pid_t client_pid;
> +static char sock_name[32];
> +
> +static void die(int status)
> +{
> +	unlink(sock_name);
> +	kill(client_pid, SIGTERM);
> +	exit(status);
> +}
> +
> +static void child_die()
> +{
> +	kill(getppid(), SIGTERM);
> +	exit(1);
> +}
> +
> +static int safe_int(const char *numstr, int *converted)
> +{
> +	char *err = NULL;
> +	long sli;
> +
> +	errno = 0;
> +	sli = strtol(numstr, &err, 0);
> +	if (errno == ERANGE && (sli == LONG_MAX || sli == LONG_MIN))
> +		return -ERANGE;
> +
> +	if (errno != 0 && sli == 0)
> +		return -EINVAL;
> +
> +	if (err == numstr || *err != '\0')
> +		return -EINVAL;
> +
> +	if (sli > INT_MAX || sli < INT_MIN)
> +		return -ERANGE;
> +
> +	*converted = (int)sli;
> +	return 0;
> +}
> +
> +static int char_left_gc(const char *buffer, size_t len)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < len; i++) {
> +		if (buffer[i] == ' ' || buffer[i] == '\t')
> +			continue;
> +
> +		return i;
> +	}
> +
> +	return 0;
> +}
> +
> +static int char_right_gc(const char *buffer, size_t len)
> +{
> +	int i;
> +
> +	for (i = len - 1; i >= 0; i--) {
> +		if (buffer[i] == ' ' || buffer[i] == '\t' ||
> +		    buffer[i] == '\n' || buffer[i] == '\0')
> +			continue;
> +
> +		return i + 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static char *trim_whitespace_in_place(char *buffer)
> +{
> +	buffer += char_left_gc(buffer, strlen(buffer));
> +	buffer[char_right_gc(buffer, strlen(buffer))] = '\0';
> +	return buffer;
> +}
> +
> +/* borrowed (with all helpers) from pidfd/pidfd_open_test.c */
> +static pid_t get_pid_from_fdinfo_file(int pidfd, const char *key, size_t keylen)
> +{
> +	int ret;
> +	char path[512];
> +	FILE *f;
> +	size_t n = 0;
> +	pid_t result = -1;
> +	char *line = NULL;
> +
> +	snprintf(path, sizeof(path), "/proc/self/fdinfo/%d", pidfd);
> +
> +	f = fopen(path, "re");
> +	if (!f)
> +		return -1;
> +
> +	while (getline(&line, &n, f) != -1) {
> +		char *numstr;
> +
> +		if (strncmp(line, key, keylen))
> +			continue;
> +
> +		numstr = trim_whitespace_in_place(line + 4);
> +		ret = safe_int(numstr, &result);
> +		if (ret < 0)
> +			goto out;
> +
> +		break;
> +	}
> +
> +out:
> +	free(line);
> +	fclose(f);
> +	return result;
> +}
> +
> +static int cmsg_check(int fd)
> +{
> +	struct msghdr msg = { 0 };
> +	struct cmsghdr *cmsg;
> +	struct iovec iov;
> +	struct ucred *ucred = NULL;
> +	int data = 0;
> +	char control[CMSG_SPACE(sizeof(struct ucred)) +
> +		     CMSG_SPACE(sizeof(int))] = { 0 };
> +	int *pidfd = NULL;
> +	pid_t parent_pid;
> +	int err;
> +
> +	iov.iov_base = &data;
> +	iov.iov_len = sizeof(data);
> +
> +	msg.msg_iov = &iov;
> +	msg.msg_iovlen = 1;
> +	msg.msg_control = control;
> +	msg.msg_controllen = sizeof(control);
> +
> +	err = recvmsg(fd, &msg, 0);
> +	if (err < 0) {
> +		log_err("recvmsg");
> +		return 1;
> +	}
> +
> +	if (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC)) {
> +		log_err("recvmsg: truncated");
> +		return 1;
> +	}
> +
> +	for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL;
> +	     cmsg = CMSG_NXTHDR(&msg, cmsg)) {
> +		if (cmsg->cmsg_level == SOL_SOCKET &&
> +		    cmsg->cmsg_type == SCM_PIDFD) {
> +			if (cmsg->cmsg_len < sizeof(*pidfd)) {
> +				log_err("CMSG parse: SCM_PIDFD wrong len");
> +				return 1;
> +			}
> +
> +			pidfd = (void *)CMSG_DATA(cmsg);
> +		}
> +
> +		if (cmsg->cmsg_level == SOL_SOCKET &&
> +		    cmsg->cmsg_type == SCM_CREDENTIALS) {
> +			if (cmsg->cmsg_len < sizeof(*ucred)) {
> +				log_err("CMSG parse: SCM_CREDENTIALS wrong len");
> +				return 1;
> +			}
> +
> +			ucred = (void *)CMSG_DATA(cmsg);
> +		}
> +	}
> +
> +	/* send(pfd, "x", sizeof(char), 0) */
> +	if (data != 'x') {
> +		log_err("recvmsg: data corruption");
> +		return 1;
> +	}
> +
> +	if (!pidfd) {
> +		log_err("CMSG parse: SCM_PIDFD not found");
> +		return 1;
> +	}
> +
> +	if (!ucred) {
> +		log_err("CMSG parse: SCM_CREDENTIALS not found");
> +		return 1;
> +	}
> +
> +	/* pidfd from SCM_PIDFD should point to the parent process PID */
> +	parent_pid =
> +		get_pid_from_fdinfo_file(*pidfd, "Pid:", sizeof("Pid:") - 1);
> +	if (parent_pid != getppid()) {
> +		log_err("wrong SCM_PIDFD %d != %d", parent_pid, getppid());
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +void client(struct sockaddr_un *listen_addr)
> +{
> +	int cfd;
> +	socklen_t len;
> +	struct ucred peer_cred;
> +	int peer_pidfd;
> +	pid_t peer_pid;
> +	int on = 0;
> +
> +	cfd = socket(AF_UNIX, SOCK_STREAM, 0);
> +	if (cfd < 0) {
> +		log_err("socket");
> +		child_die();
> +	}
> +
> +	if (connect(cfd, (struct sockaddr *)listen_addr,
> +		    sizeof(*listen_addr)) != 0) {
> +		log_err("connect");
> +		child_die();
> +	}
> +
> +	on = 1;
> +	if (setsockopt(cfd, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on))) {
> +		log_err("Failed to set SO_PASSCRED");
> +		child_die();
> +	}
> +
> +	if (setsockopt(cfd, SOL_SOCKET, SO_PASSPIDFD, &on, sizeof(on))) {
> +		log_err("Failed to set SO_PASSPIDFD");
> +		child_die();
> +	}
> +
> +	if (cmsg_check(cfd)) {
> +		log_err("cmsg_check failed");
> +		child_die();
> +	}
> +
> +	len = sizeof(peer_cred);
> +	if (getsockopt(cfd, SOL_SOCKET, SO_PEERCRED, &peer_cred, &len)) {
> +		log_err("Failed to get SO_PEERCRED");
> +		child_die();
> +	}
> +
> +	len = sizeof(peer_pidfd);
> +	if (getsockopt(cfd, SOL_SOCKET, SO_PEERPIDFD, &peer_pidfd, &len)) {
> +		log_err("Failed to get SO_PEERPIDFD");
> +		child_die();
> +	}
> +
> +	/* pid from SO_PEERCRED should point to the parent process PID */
> +	if (peer_cred.pid != getppid()) {
> +		log_err("Failed to get SO_PEERPIDFD");
> +		child_die();
> +	}
> +
> +	peer_pid = get_pid_from_fdinfo_file(peer_pidfd,
> +					    "Pid:", sizeof("Pid:") - 1);
> +	if (peer_pid != peer_cred.pid) {
> +		log_err("Failed to get SO_PEERPIDFD");
> +		child_die();
> +	}
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int lfd, pfd;
> +	int child_status = 0;
> +	struct sockaddr_un listen_addr;
> +
> +	lfd = socket(AF_UNIX, SOCK_STREAM, 0);
> +	if (lfd < 0) {
> +		perror("socket");
> +		exit(1);
> +	}
> +
> +	memset(&listen_addr, 0, sizeof(listen_addr));
> +	listen_addr.sun_family = AF_UNIX;
> +	sprintf(sock_name, "scm_pidfd_%d", getpid());
> +	unlink(sock_name);
> +	strcpy(listen_addr.sun_path, sock_name);
> +
> +	if ((bind(lfd, (struct sockaddr *)&listen_addr, sizeof(listen_addr))) !=
> +	    0) {
> +		perror("socket bind failed");
> +		exit(1);
> +	}
> +
> +	if (listen(lfd, 1) < 0) {
> +		perror("listen");
> +		exit(1);
> +	}
> +
> +	client_pid = fork();
> +	if (client_pid < 0) {
> +		perror("fork");
> +		exit(1);
> +	}
> +
> +	if (client_pid == 0) {
> +		client(&listen_addr);
> +		exit(0);
> +	}
> +
> +	pfd = accept(lfd, NULL, NULL);
> +	if (pfd < 0) {
> +		perror("accept");
> +		die(1);
> +	}
> +
> +	if (send(pfd, "x", sizeof(char), 0) < 0) {
> +		perror("send");
> +		die(1);
> +	}
> +
> +	waitpid(client_pid, &child_status, 0);
> +	die(WIFEXITED(child_status) ? WEXITSTATUS(child_status) : 1);
> +	die(0);
> +}
> \ No newline at end of file
> -- 
> 2.34.1

Powered by blists - more mailing lists