From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: <aleksandr.mikhalitsyn@canonical.com>
Cc: <arnd@arndb.de>, <brauner@kernel.org>, <davem@davemloft.net>,
<dsahern@kernel.org>, <edumazet@google.com>,
<keescook@chromium.org>, <kuba@kernel.org>, <leon@kernel.org>,
<linux-arch@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-kselftest@vger.kernel.org>, <netdev@vger.kernel.org>,
<pabeni@redhat.com>
Subject: Re: [PATCH net-next v2 3/3] selftests: net: add SCM_PIDFD / SO_PEERPIDFD test
Date: Tue, 21 Mar 2023 17:47:52 -0700 [thread overview]
Message-ID: <20230322004752.30055-1-kuniyu@amazon.com> (raw)
In-Reply-To: <20230321183342.617114-4-aleksandr.mikhalitsyn@canonical.com>
From: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.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@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: David Ahern <dsahern@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: linux-kselftest@vger.kernel.org
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.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
next prev parent reply other threads:[~2023-03-22 0:48 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-21 18:33 [PATCH net-next v2 0/3] Add SCM_PIDFD and SO_PEERPIDFD Alexander Mikhalitsyn
2023-03-21 18:33 ` [PATCH net-next v2 1/3] scm: add SO_PASSPIDFD and SCM_PIDFD Alexander Mikhalitsyn
2023-03-22 0:43 ` Kuniyuki Iwashima
2023-03-22 13:41 ` kernel test robot
2023-03-22 15:48 ` Christian Brauner
2023-03-21 18:33 ` [PATCH net-next v2 2/3] net: core: add getsockopt SO_PEERPIDFD Alexander Mikhalitsyn
2023-03-22 0:44 ` Kuniyuki Iwashima
2023-03-22 15:35 ` Christian Brauner
2023-03-22 16:16 ` Aleksandr Mikhalitsyn
2023-03-28 15:45 ` Christian Brauner
2023-03-29 6:43 ` [PATCH 0/3] pidfd: add pidfd_prepare() Christian Brauner
2023-03-21 18:33 ` [PATCH net-next v2 3/3] selftests: net: add SCM_PIDFD / SO_PEERPIDFD test Alexander Mikhalitsyn
2023-03-22 0:47 ` Kuniyuki Iwashima [this message]
2023-03-22 14:13 ` [PATCH net-next v2 0/3] Add SCM_PIDFD and SO_PEERPIDFD Christian Brauner
2023-03-22 14:17 ` Aleksandr Mikhalitsyn
2023-03-27 18:22 [PATCH 0/3] pidfd: add pidfd_prepare() Christian Brauner
2023-03-27 18:22 ` [PATCH 1/3] pid: " Christian Brauner
2023-03-28 9:00 ` Jan Kara
2023-03-27 18:22 ` [PATCH 2/3] fork: use pidfd_prepare() Christian Brauner
2023-03-27 18:22 ` [PATCH 3/3] fanotify: " Christian Brauner
2023-03-28 7:54 ` Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230322004752.30055-1-kuniyu@amazon.com \
--to=kuniyu@amazon.com \
--cc=aleksandr.mikhalitsyn@canonical.com \
--cc=arnd@arndb.de \
--cc=brauner@kernel.org \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=keescook@chromium.org \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.