Netdev List
 help / color / mirror / Atom feed
From: Jori Koolstra <jkoolstra@xs4all.nl>
To: Kuniyuki Iwashima <kuniyu@google.com>
Cc: brauner@kernel.org, cyphar@cyphar.com, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	horms@kernel.org, netdev@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v6 3/4] net: af_unix: useful handling of LSM denials on SCM_RIGHTS
Date: Wed, 5 Aug 2026 12:09:12 +0200 (CEST)	[thread overview]
Message-ID: <728282285.265641.1785924552245@kpc.webmail.kpnmail.nl> (raw)
In-Reply-To: <CAAVpQUCo0r+DDRJ9BT1_2JYmPcsUNnk6aS+oqPvOrO9ruCPUKw@mail.gmail.com>


> Op 04-08-2026 19:16 CEST schreef Kuniyuki Iwashima <kuniyu@google.com>:
> 
>  
> On Sun, Aug 2, 2026 at 8:11 AM Jori Koolstra <jkoolstra@xs4all.nl> 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@kernel.org>
> > Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> > ---
> >  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
> >

  reply	other threads:[~2026-08-05 10:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-02 15:12 [PATCH net-next v6 0/4] net: af_unix: useful handling of LSM denials on SCM_RIGHTS Jori Koolstra
2026-08-02 15:12 ` [PATCH net-next v6 1/4] net: af_unix: enable custom setsockopt for all socket types Jori Koolstra
2026-08-04 16:49   ` Kuniyuki Iwashima
2026-08-02 15:12 ` [PATCH net-next v6 2/4] net: scm: move scm_detach_fds() from common path to scm_recv_unix() Jori Koolstra
2026-08-02 15:12 ` [PATCH net-next v6 3/4] net: af_unix: useful handling of LSM denials on SCM_RIGHTS Jori Koolstra
2026-08-04 17:16   ` Kuniyuki Iwashima
2026-08-05 10:09     ` Jori Koolstra [this message]
2026-08-05 16:27       ` Kuniyuki Iwashima
2026-08-02 15:12 ` [PATCH net-next v6 4/4] selftest: Add tests for " Jori Koolstra
2026-08-04 17:26   ` Kuniyuki Iwashima
2026-08-05 10:18     ` Jori Koolstra
2026-08-05 16:30       ` Kuniyuki Iwashima

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=728282285.265641.1785924552245@kpc.webmail.kpnmail.nl \
    --to=jkoolstra@xs4all.nl \
    --cc=brauner@kernel.org \
    --cc=cyphar@cyphar.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox