Re: [PATCH net-next v6 3/4] net: af_unix: useful handling of LSM denials on SCM_RIGHTS
Jori Koolstra <[email protected]> Wed, 5 Aug 2026 12:09:12 +0200 (CEST)
| Newsgroups | gmane.linux.file-systems,gmane.linux.network,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
> Op 04-08-2026 19:16 CEST schreef Kuniyuki Iwashima <[email protected]>: > > > On Sun, Aug 2, 2026 at 8:11 AM Jori Koolstra <[email protected]> wrote: > > > > Right now if some LSM such as Smack denies an AF_UNIX socket peer to > > receive an SCM_RIGHTS fd, the SCM_RIGHTS fd array will be cut short at > > that point, and MSG_CTRUNC is set on return of recvmsg(). This is > > highly problematic behaviour, because it leaves the receiver > > wondering what happened. As per man page MSG_CTRUNC is supposed to > > indicate that the control buffer was sized too short, but suddenly > > a permission error might result in the exact same flag being set. > > Moreover, the receiver has no chance to determine how many fds got > > originally sent and how many were suppressed.[1] > > > > Add a SO_RIGHTS_NOTRUNC option to UNIX sockets to enable more useful > > handling of LSM denials when receiving SCM_RIGHTS messages: instead of > > truncating the message at the first blocked fd, keep every fd slot > > and store the LSM errno in the blocked slot. The socket option is > > inherited by the child accept() socket if set on the listen() socket. > > > > [1]: https://github.com/uapi-group/kernel-features#useful-handling-of-lsm-denials-on-scm_rights > > > > Reviewed-by: Christian Brauner (Amutable) <[email protected]> > > Signed-off-by: Jori Koolstra <[email protected]> > > --- > > 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/af_unix.h | 1 + > > include/net/scm.h | 13 +++------ > > include/uapi/asm-generic/socket.h | 2 ++ > > net/compat.c | 4 +-- > > net/core/scm.c | 38 +++++++++++++++++++++++---- > > net/unix/af_unix.c | 12 ++++++++- > > 10 files changed, 61 insertions(+), 17 deletions(-) > > > > diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h > > index 5ef57f88df6b..946a5fad2691 100644 > > --- a/arch/alpha/include/uapi/asm/socket.h > > +++ b/arch/alpha/include/uapi/asm/socket.h > > @@ -155,6 +155,8 @@ > > #define SO_INQ 84 > > #define SCM_INQ SO_INQ > > > > +#define SO_RIGHTS_NOTRUNC 85 > > + > > #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 72fb1b006da9..f1641dde135f 100644 > > --- a/arch/mips/include/uapi/asm/socket.h > > +++ b/arch/mips/include/uapi/asm/socket.h > > @@ -166,6 +166,8 @@ > > #define SO_INQ 84 > > #define SCM_INQ SO_INQ > > > > +#define SO_RIGHTS_NOTRUNC 85 > > + > > #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 c16ec36dfee6..f3a3815c7dc2 100644 > > --- a/arch/parisc/include/uapi/asm/socket.h > > +++ b/arch/parisc/include/uapi/asm/socket.h > > @@ -147,6 +147,8 @@ > > #define SO_INQ 0x4052 > > #define SCM_INQ SO_INQ > > > > +#define SO_RIGHTS_NOTRUNC 0x4053 > > + > > #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 71befa109e1c..7907f3b1f0ee 100644 > > --- a/arch/sparc/include/uapi/asm/socket.h > > +++ b/arch/sparc/include/uapi/asm/socket.h > > @@ -148,6 +148,8 @@ > > #define SO_INQ 0x005d > > #define SCM_INQ SO_INQ > > > > +#define SO_RIGHTS_NOTRUNC 0x005e > > + > > #if !defined(__KERNEL__) > > > > > > diff --git a/include/net/af_unix.h b/include/net/af_unix.h > > index 34f53dde65ce..bb1b3dee02e8 100644 > > --- a/include/net/af_unix.h > > +++ b/include/net/af_unix.h > > @@ -49,6 +49,7 @@ struct unix_sock { > > struct scm_stat scm_stat; > > int inq_len; > > bool recvmsg_inq; > > + bool scm_rights_notrunc; > > #if IS_ENABLED(CONFIG_AF_UNIX_OOB) > > struct sk_buff *oob_skb; > > #endif > > diff --git a/include/net/scm.h b/include/net/scm.h > > index c52519669349..86ae6bc109ec 100644 > > --- a/include/net/scm.h > > +++ b/include/net/scm.h > > @@ -50,8 +50,8 @@ struct scm_cookie { > > #endif > > }; > > > > -void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm); > > -void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm); > > +void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm, bool notrunc); > > +void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm, bool notrunc); > > int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm); > > void __scm_destroy(struct scm_cookie *scm); > > struct scm_fp_list *scm_fp_dup(struct scm_fp_list *fpl); > > @@ -107,13 +107,8 @@ void scm_recv(struct socket *sock, struct msghdr *msg, > > void scm_recv_unix(struct socket *sock, struct msghdr *msg, > > struct scm_cookie *scm, int flags); > > > > -static inline int scm_recv_one_fd(struct file *f, int __user *ufd, > > - unsigned int flags) > > -{ > > - if (!ufd) > > - return -EFAULT; > > - return receive_fd(f, ufd, flags); > > -} > > +int scm_recv_one_fd(struct file *f, int __user *ufd, unsigned int flags, > > + bool notrunc); > > > > #endif /* __LINUX_NET_SCM_H */ > > > > diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h > > index 53b5a8c002b1..84ea7b92936e 100644 > > --- a/include/uapi/asm-generic/socket.h > > +++ b/include/uapi/asm-generic/socket.h > > @@ -150,6 +150,8 @@ > > #define SO_INQ 84 > > #define SCM_INQ SO_INQ > > > > +#define SO_RIGHTS_NOTRUNC 85 > > + > > #if !defined(__KERNEL__) > > > > #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__)) > > diff --git a/net/compat.c b/net/compat.c > > index d68cf9c3aad5..6bdf4a2c9077 100644 > > --- a/net/compat.c > > +++ b/net/compat.c > > @@ -286,7 +286,7 @@ static int scm_max_fds_compat(struct msghdr *msg) > > return (msg->msg_controllen - sizeof(struct compat_cmsghdr)) / sizeof(int); > > } > > > > -void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) > > +void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm, bool notrunc) > > { > > struct compat_cmsghdr __user *cm = > > (struct compat_cmsghdr __user *)msg->msg_control_user; > > @@ -296,7 +296,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) > > int err = 0, i; > > > > for (i = 0; i < fdmax; i++) { > > - err = scm_recv_one_fd(scm->fp->fp[i], cmsg_data + i, o_flags); > > + err = scm_recv_one_fd(scm->fp->fp[i], cmsg_data + i, o_flags, notrunc); > > if (err < 0) > > break; > > } > > diff --git a/net/core/scm.c b/net/core/scm.c > > index a73b1eb30fd2..f0d44ecdb11f 100644 > > --- a/net/core/scm.c > > +++ b/net/core/scm.c > > @@ -351,7 +351,31 @@ static int scm_max_fds(struct msghdr *msg) > > return (msg->msg_controllen - sizeof(struct cmsghdr)) / sizeof(int); > > } > > > > -void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) > > +int scm_recv_one_fd(struct file *f, int __user *ufd, unsigned int flags, > > + bool notrunc) > > +{ > > + int error; > > + > > + if (!ufd) > > + return -EFAULT; > > + > > + error = security_file_receive(f); > > + if (error) > > + return notrunc ? put_user(error, ufd) : error; > > + > > + FD_PREPARE(fdf, flags, get_file(f)); > > + if (fdf.err) > > + return fdf.err; > > + > > + error = put_user(fd_prepare_fd(fdf), ufd); > > + if (error) > > + return error; > > + > > + __receive_sock(fd_prepare_file(fdf)); > > + return fd_publish(fdf); > > +} > > + > > +void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm, bool notrunc) > > { > > struct cmsghdr __user *cm = > > (__force struct cmsghdr __user *)msg->msg_control_user; > > @@ -365,12 +389,12 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) > > return; > > > > if (msg->msg_flags & MSG_CMSG_COMPAT) { > > - scm_detach_fds_compat(msg, scm); > > + scm_detach_fds_compat(msg, scm, notrunc); > > return; > > } > > > > for (i = 0; i < fdmax; i++) { > > - err = scm_recv_one_fd(scm->fp->fp[i], cmsg_data + i, o_flags); > > + err = scm_recv_one_fd(scm->fp->fp[i], cmsg_data + i, o_flags, notrunc); > > if (err < 0) > > break; > > } > > @@ -542,8 +566,12 @@ void scm_recv_unix(struct socket *sock, struct msghdr *msg, > > if (!__scm_recv_common(sock->sk, msg, scm, flags)) > > return; > > > > - if (scm->fp) > > - scm_detach_fds(msg, scm); > > + if (scm->fp) { > > + struct unix_sock *u; > > + > > + u = unix_sk(sock->sk); > > + scm_detach_fds(msg, scm, READ_ONCE(u->scm_rights_notrunc)); > > + } > > > > if (sock->sk->sk_scm_pidfd) > > scm_pidfd_recv(msg, scm); > > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c > > index 51cbf920130d..03ce23a4ebee 100644 > > --- a/net/unix/af_unix.c > > +++ b/net/unix/af_unix.c > > @@ -922,6 +922,7 @@ static bool unix_custom_sockopt(int optname) > > { > > switch (optname) { > > case SO_INQ: > > + case SO_RIGHTS_NOTRUNC: > > return true; > > default: > > return false; > > @@ -957,6 +958,14 @@ static int unix_setsockopt(struct socket *sock, int level, int optname, > > > > WRITE_ONCE(u->recvmsg_inq, val); > > break; > > + > > + case SO_RIGHTS_NOTRUNC: > > + if (val > 1 || val < 0) > > + return -EINVAL; > > + > > + WRITE_ONCE(u->scm_rights_notrunc, val); > > + break; > > + > > default: > > return -ENOPROTOOPT; > > } > > @@ -1746,9 +1755,10 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad > > init_peercred(newsk, &peercred); > > > > newu = unix_sk(newsk); > > + otheru = unix_sk(other); > > newu->listener = other; > > + newu->scm_rights_notrunc = otheru->scm_rights_notrunc; > > nit: READ_ONCE() is needed here. > Yeah, I was thinking whether to add that or not. We have newsk->sk_scm_recv_flags = other->sk_scm_recv_flags; earlier, so without READ_ONCE(), and earlier there is unix_state_lock(other); which should make this assignment safe. However, that does not protect necessarily against compiler mangling. Then again, we are reading a bool here, so isn't this a theoretical concern mostly? I am happy to change it though (if only to signal intent), but then the access to sk_scm_recv_flags should also be READ_ONCE(), I believe, or it will be needlessly confusing why it is there for one access but not the other. Thanks, Jori. > > > > RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq); > > - otheru = unix_sk(other); > > > > /* copy address information from listening to new sock > > * > > -- > > 2.55.0 > >