[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241116011038.94912-1-kuniyu@amazon.com>
Date: Fri, 15 Nov 2024 17:10:38 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <stsp2@...dex.ru>
CC: <almasrymina@...gle.com>, <asml.silence@...il.com>, <axboe@...nel.dk>,
<brauner@...nel.org>, <cyphar@...har.com>, <davem@...emloft.net>,
<edumazet@...gle.com>, <gouhao@...ontech.com>, <horms@...nel.org>,
<kees@...nel.org>, <krisman@...e.de>, <kuba@...nel.org>, <kuniyu@...zon.com>,
<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
<mhal@...x.co>, <netdev@...r.kernel.org>, <oleg@...hat.com>,
<pabeni@...hat.com>, <quic_abchauha@...cinc.com>, <shuah@...nel.org>,
<tandersen@...flix.com>, <viro@...iv.linux.org.uk>, <willemb@...gle.com>
Subject: Re: [PATCH v2] net/unix: pass pidfd flags via SCM_PIDFD cmsg
> [PATCH v2] net/unix: pass pidfd flags via SCM_PIDFD cmsg
Please specify the target tree; net for fixes, net-next for others.
https://www.kernel.org/doc/html/v6.11/process/maintainer-netdev.html
[PATCH net-next v3] af_unix: pass ...
From: Stas Sergeev <stsp2@...dex.ru>
Date: Thu, 14 Nov 2024 12:19:09 +0300
> Currently SCM_PIDFD cmsg cannot be sent via unix socket
> (returns -EINVAL) and SO_PASSPIDFD doesn't support flags.
> The created pidfd always has flags set to 0.
>
> This patch implements SCM_PIDFD cmsg in AF_UNIX socket, which
> can be used to send flags to SO_PASSPIDFD-enabled recipient.
>
> Self-test is added for the propagation of PIDFD_NONBLOCK flag.
>
> This is mainly needed for the future extensions, like eg this one:
> https://lore.kernel.org/lkml/8288a08e-448b-43c2-82dc-59f87d0d9072@yandex.ru/T/#me1237e46deba8574b77834b7704e63559ffef9cb
> where it was suggested to try solving the supplementary groups
> problem with pidfd.
>
> Changes in v2: remove flags validation in scm_pidfd_recv(), as
> suggested by Kuniyuki Iwashima <kuniyu@...zon.com>
You can put this changelog and the following CC: under '---' so
that they will disappear during merge.
>
> Signed-off-by: Stas Sergeev <stsp2@...dex.ru>
>
> 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: Simon Horman <horms@...nel.org>
> CC: Shuah Khan <shuah@...nel.org>
> CC: Christian Brauner <brauner@...nel.org>
> CC: Jens Axboe <axboe@...nel.dk>
> CC: Willem de Bruijn <willemb@...gle.com>
> CC: Pavel Begunkov <asml.silence@...il.com>
> CC: Gabriel Krisman Bertazi <krisman@...e.de>
> CC: Mina Almasry <almasrymina@...gle.com>
> CC: Oleg Nesterov <oleg@...hat.com>
> CC: Tycho Andersen <tandersen@...flix.com>
> CC: Al Viro <viro@...iv.linux.org.uk>
> CC: Kuniyuki Iwashima <kuniyu@...zon.com>
> CC: Gou Hao <gouhao@...ontech.com>
> CC: Abhishek Chauhan <quic_abchauha@...cinc.com>
> CC: Michal Luczaj <mhal@...x.co>
> CC: Kees Cook <kees@...nel.org>
> CC: Aleksa Sarai <cyphar@...har.com>
> CC: linux-kernel@...r.kernel.org
> CC: netdev@...r.kernel.org
> CC: linux-kselftest@...r.kernel.org
> ---
^^^ Here
> include/linux/pidfs.h | 9 +++
> include/linux/socket.h | 2 +-
> include/net/af_unix.h | 1 +
> include/net/scm.h | 3 +-
> kernel/pid.c | 6 +-
> net/core/scm.c | 14 ++++
> net/core/sock.c | 1 +
> net/unix/af_unix.c | 3 +
> .../testing/selftests/net/af_unix/scm_pidfd.c | 70 +++++++++++++++++--
> 9 files changed, 99 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/pidfs.h b/include/linux/pidfs.h
> index 75bdf9807802..c4c5c1a0c2ad 100644
> --- a/include/linux/pidfs.h
> +++ b/include/linux/pidfs.h
> @@ -2,7 +2,16 @@
> #ifndef _LINUX_PID_FS_H
> #define _LINUX_PID_FS_H
>
> +#include <uapi/linux/pidfd.h>
> +
> struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags);
> void __init pidfs_init(void);
>
> +static inline int pidfd_validate_flags(unsigned int flags)
> +{
> + if (flags & ~(PIDFD_NONBLOCK | PIDFD_THREAD))
> + return -EINVAL;
> + return 0;
> +}
> +
> #endif /* _LINUX_PID_FS_H */
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index d18cc47e89bd..ee27d391e5aa 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -178,7 +178,7 @@ static inline size_t msg_data_left(struct msghdr *msg)
> #define SCM_RIGHTS 0x01 /* rw: access rights (array of int) */
> #define SCM_CREDENTIALS 0x02 /* rw: struct ucred */
> #define SCM_SECURITY 0x03 /* rw: security label */
> -#define SCM_PIDFD 0x04 /* ro: pidfd (int) */
> +#define SCM_PIDFD 0x04 /* r: pidfd, w: pidfd_flags (int) */
>
> struct ucred {
> __u32 pid;
> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
> index 63129c79b8cb..4bc197548c2f 100644
> --- a/include/net/af_unix.h
> +++ b/include/net/af_unix.h
> @@ -62,6 +62,7 @@ struct unix_skb_parms {
> #ifdef CONFIG_SECURITY_NETWORK
> u32 secid; /* Security ID */
> #endif
> + u32 pidfd_flags;
> u32 consumed;
> } __randomize_layout;
>
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 0d35c7c77a74..1326edcacacb 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -48,6 +48,7 @@ struct scm_cookie {
> #ifdef CONFIG_SECURITY_NETWORK
> u32 secid; /* Passed security ID */
> #endif
> + u32 pidfd_flags;
Now we consume 40 byes of cb[48].
If we need more storage in the future, we may want to save
converted flags in __scm_send() and restore that in
scm_pidfd_recv().
No need to do so now, just a note.
> };
>
> void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm);
> @@ -154,7 +155,7 @@ static __inline__ void scm_pidfd_recv(struct msghdr *msg, struct scm_cookie *scm
> if (!scm->pid)
> return;
>
> - pidfd = pidfd_prepare(scm->pid, 0, &pidfd_file);
> + pidfd = pidfd_prepare(scm->pid, scm->pidfd_flags, &pidfd_file);
>
> if (put_cmsg(msg, SOL_SOCKET, SCM_PIDFD, sizeof(int), &pidfd)) {
> if (pidfd_file) {
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 2715afb77eab..b1100ae8ea63 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -629,10 +629,12 @@ static int pidfd_create(struct pid *pid, unsigned int flags)
> SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags)
> {
> int fd;
> + int err;
> struct pid *p;
>
> - if (flags & ~(PIDFD_NONBLOCK | PIDFD_THREAD))
> - return -EINVAL;
> + err = pidfd_validate_flags(flags);
> + if (err)
> + return err;
>
> if (pid <= 0)
> return -EINVAL;
> diff --git a/net/core/scm.c b/net/core/scm.c
> index 4f6a14babe5a..3bcdecdacd7e 100644
> --- a/net/core/scm.c
> +++ b/net/core/scm.c
> @@ -23,6 +23,7 @@
> #include <linux/security.h>
> #include <linux/pid_namespace.h>
> #include <linux/pid.h>
> +#include <linux/pidfs.h>
> #include <linux/nsproxy.h>
> #include <linux/slab.h>
> #include <linux/errqueue.h>
> @@ -210,6 +211,19 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
> p->creds.gid = gid;
> break;
> }
> + case SCM_PIDFD:
> + {
> + unsigned int flags;
> +
> + if (cmsg->cmsg_len != CMSG_LEN(sizeof(flags)))
> + goto error;
> + memcpy(&flags, CMSG_DATA(cmsg), sizeof(flags));
> + err = pidfd_validate_flags(flags);
> + if (err)
> + goto error;
> + p->pidfd_flags = flags;
> + break;
> + }
Now this allows sending pidfd without SO_PASSPIDFD, so you need to
add a validation for "if (!msg->msg_control)" in __scm_recv_common().
> default:
> goto error;
> }
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 039be95c40cf..d1fce437c035 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2930,6 +2930,7 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
> /* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */
> case SCM_RIGHTS:
> case SCM_CREDENTIALS:
> + case SCM_PIDFD:
> break;
> default:
> return -EINVAL;
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 001ccc55ef0f..8b19dfec0221 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1892,6 +1892,7 @@ static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool sen
> UNIXCB(skb).uid = scm->creds.uid;
> UNIXCB(skb).gid = scm->creds.gid;
> UNIXCB(skb).fp = NULL;
> + UNIXCB(skb).pidfd_flags = scm->pidfd_flags;
> unix_get_secdata(scm, skb);
> if (scm->fp && send_fds)
> err = unix_attach_fds(scm, skb);
> @@ -2486,6 +2487,7 @@ int __unix_dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t size,
> memset(&scm, 0, sizeof(scm));
>
> scm_set_cred(&scm, UNIXCB(skb).pid, UNIXCB(skb).uid, UNIXCB(skb).gid);
> + scm.pidfd_flags = UNIXCB(skb).pidfd_flags;
> unix_set_secdata(&scm, skb);
>
> if (!(flags & MSG_PEEK)) {
> @@ -2873,6 +2875,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
> test_bit(SOCK_PASSPIDFD, &sock->flags)) {
> /* Copy credentials */
> scm_set_cred(&scm, UNIXCB(skb).pid, UNIXCB(skb).uid, UNIXCB(skb).gid);
> + scm.pidfd_flags = UNIXCB(skb).pidfd_flags;
> unix_set_secdata(&scm, skb);
> check_creds = true;
> }
Powered by blists - more mailing lists