Re: [PATCH net-next v6 3/4] net: af_unix: useful handling of LSM denials on SCM_RIGHTS

[Kuniyuki Iwashima]

On Wed, Aug 5, 2026 at 3:09 AM Jori Koolstra <jkoolstra@xxxxxxxxx> wrote:
>
>
> > Op 04-08-2026 19:16 CEST schreef Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>:
> >
> >
> > On Sun, Aug 2, 2026 at 8:11 AM Jori Koolstra <jkoolstra@xxxxxxxxx> 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) <brauner@xxxxxxxxxx>
> > > Signed-off-by: Jori Koolstra <jkoolstra@xxxxxxxxx>
> > > ---
> > >  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.

unix_setsockopt() is called without any lock, and unix_state_lock()
does not protect the values, so that's why we need WRITE_ONCE()
and READ_ONCE().

> 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?

Yes, but KCSAN will detect it (and syzbot will complain).


>
> 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(),

This has a bit different context as its actual value, each
sk_scm_XXX in union, is a bit field, so we don't need to care
about any load-tearing even though the writer is under yet another
lock, lock_sock().


> 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
> > >