[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <68263fc569b91_25ebe529448@willemb.c.googlers.com.notmuch>
Date: Thu, 15 May 2025 15:25:57 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Willem de Bruijn <willemb@...gle.com>
Cc: Simon Horman <horms@...nel.org>,
Christian Brauner <brauner@...nel.org>,
Kuniyuki Iwashima <kuniyu@...zon.com>,
Kuniyuki Iwashima <kuni1840@...il.com>,
netdev@...r.kernel.org
Subject: Re: [PATCH v3 net-next 8/9] af_unix: Introduce SO_PASSRIGHTS.
Kuniyuki Iwashima wrote:
> As long as recvmsg() or recvmmsg() is used with cmsg, it is not
> possible to avoid receiving file descriptors via SCM_RIGHTS.
>
> This behaviour has occasionally been flagged as problematic, as
> it can be (ab)used to trigger DoS during close(), for example, by
> passing a FUSE-controlled fd or a hung NFS fd.
>
> For instance, as noted on the uAPI Group page [0], an untrusted peer
> could send a file descriptor pointing to a hung NFS mount and then
> close it. Once the receiver calls recvmsg() with msg_control, the
> descriptor is automatically installed, and then the responsibility
> for the final close() now falls on the receiver, which may result
> in blocking the process for a long time.
>
> Regarding this, systemd calls cmsg_close_all() [1] after each
> recvmsg() to close() unwanted file descriptors sent via SCM_RIGHTS.
>
> However, this cannot work around the issue at all, because the final
> fput() may still occur on the receiver's side once sendmsg() with
> SCM_RIGHTS succeeds. Also, even filtering by LSM at recvmsg() does
> not work for the same reason.
>
> Thus, we need a better way to refuse SCM_RIGHTS at sendmsg().
>
> Let's introduce SO_PASSRIGHTS to disable SCM_RIGHTS.
>
> Note that this option is enabled by default for backward
> compatibility.
>
> Link: https://uapi-group.org/kernel-features/#disabling-reception-of-scm_rights-for-af_unix-sockets #[0]
> Link: https://github.com/systemd/systemd/blob/v257.5/src/basic/fd-util.c#L612-L628 #[1]
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
> v3: Return -EOPNOTSUPP for getsockopt()
> ---
> arch/alpha/include/uapi/asm/socket.h | 2 ++
> arch/mips/include/uapi/asm/socket.h | 2 ++
> arch/parisc/include/uapi/asm/socket.h | 2 ++
> arch/sparc/include/uapi/asm/socket.h | 2 ++
> include/net/sock.h | 4 +++-
> include/uapi/asm-generic/socket.h | 2 ++
> net/core/sock.c | 14 ++++++++++++++
> net/unix/af_unix.c | 22 ++++++++++++++++++++--
> tools/include/uapi/asm-generic/socket.h | 2 ++
> 9 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> index 3df5f2dd4c0f..8f1f18adcdb5 100644
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> @@ -150,6 +150,8 @@
>
> #define SO_RCVPRIORITY 82
>
> +#define SO_PASSRIGHTS 83
> +
> #if !defined(__KERNEL__)
>
> #if __BITS_PER_LONG == 64
> diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
> index 22fa8f19924a..31ac655b7837 100644
> --- a/arch/mips/include/uapi/asm/socket.h
> +++ b/arch/mips/include/uapi/asm/socket.h
> @@ -161,6 +161,8 @@
>
> #define SO_RCVPRIORITY 82
>
> +#define SO_PASSRIGHTS 83
> +
> #if !defined(__KERNEL__)
>
> #if __BITS_PER_LONG == 64
> diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
> index 96831c988606..1f2d5b7a7f5d 100644
> --- a/arch/parisc/include/uapi/asm/socket.h
> +++ b/arch/parisc/include/uapi/asm/socket.h
> @@ -142,6 +142,8 @@
> #define SCM_DEVMEM_DMABUF SO_DEVMEM_DMABUF
> #define SO_DEVMEM_DONTNEED 0x4050
>
> +#define SO_PASSRIGHTS 0x4051
> +
> #if !defined(__KERNEL__)
>
> #if __BITS_PER_LONG == 64
> diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
> index 5b464a568664..adcba7329386 100644
> --- a/arch/sparc/include/uapi/asm/socket.h
> +++ b/arch/sparc/include/uapi/asm/socket.h
> @@ -143,6 +143,8 @@
>
> #define SO_RCVPRIORITY 0x005b
>
> +#define SO_PASSRIGHTS 0x005c
> +
> #if !defined(__KERNEL__)
>
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 77232a098934..17fb6b8c4b6e 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -341,6 +341,7 @@ struct sk_filter;
> * @sk_scm_credentials: flagged by SO_PASSCRED to recv SCM_CREDENTIALS
> * @sk_scm_security: flagged by SO_PASSSEC to recv SCM_SECURITY
> * @sk_scm_pidfd: flagged by SO_PASSPIDFD to recv SCM_PIDFD
> + * @sk_scm_rights: flagged by SO_PASSRIGHTS to recv SCM_RIGHTS
> * @sk_scm_unused: unused flags for scm_recv()
> * @ns_tracker: tracker for netns reference
> * @sk_user_frags: xarray of pages the user is holding a reference on.
> @@ -534,7 +535,8 @@ struct sock {
> u8 sk_scm_credentials : 1,
> sk_scm_security : 1,
> sk_scm_pidfd : 1,
> - sk_scm_unused : 5;
> + sk_scm_rights : 1,
> + sk_scm_unused : 4;
> };
> u8 sk_clockid;
> u8 sk_txtime_deadline_mode : 1,
> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index aa5016ff3d91..f333a0ac4ee4 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -145,6 +145,8 @@
>
> #define SO_RCVPRIORITY 82
>
> +#define SO_PASSRIGHTS 83
> +
> #if !defined(__KERNEL__)
>
> #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 381abf8f25b7..0cb52e590094 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1571,6 +1571,13 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
> ret = -EOPNOTSUPP;
> break;
>
> + case SO_PASSRIGHTS:
> + if (sk_is_unix(sk))
> + sk->sk_scm_rights = valbool;
> + else
> + ret = -EOPNOTSUPP;
> + break;
> +
> case SO_INCOMING_CPU:
> reuseport_update_incoming_cpu(sk, val);
> break;
> @@ -1879,6 +1886,13 @@ int sk_getsockopt(struct sock *sk, int level, int optname,
> v.val = sk->sk_scm_pidfd;
> break;
>
> + case SO_PASSRIGHTS:
> + if (!sk_is_unix(sk))
> + return -EOPNOTSUPP;
> +
> + v.val = sk->sk_scm_rights;
Same question about lockless reading of the field.
Powered by blists - more mailing lists