Re: [PATCH v2] selinux: avoid sk_socket dereference in selinux_sctp_bind_connect()

[Stephen Smalley]

On Mon, Jun 22, 2026 at 5:03 PM Tristan Madani <tristmd@xxxxxxxxx> wrote:
>
> From: Tristan Madani <tristan@xxxxxxxxxxxxxxxxxxx>
>
> selinux_sctp_bind_connect() dereferences sk->sk_socket to pass a
> struct socket * to selinux_socket_bind() and
> selinux_socket_connect_helper().  However, when the hook is invoked
> from the ASCONF softirq path (sctp_process_asconf), there is no file
> reference guaranteeing that sk->sk_socket is non-NULL.  The setsockopt
> callers (bindx, connectx, set_primary, sendmsg connect) hold a file
> reference and are not affected.
>
> Both selinux_socket_bind() and selinux_socket_connect_helper()
> immediately resolve sock->sk, never using the struct socket * for
> anything else.  Refactor the inner logic into helpers that take a
> struct sock * directly so that selinux_sctp_bind_connect() never needs
> to touch sk->sk_socket at all.
>
> Suggested-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Tristan Madani <tristan@xxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v2:
> - Refactor selinux_socket_bind() and selinux_socket_connect_helper()
>   into sk-based inner helpers instead of adding a NULL check on
>   sk->sk_socket (Stephen Smalley)
>
>  security/selinux/hooks.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1a713d96206f..aa58a17da219 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4994,9 +4994,8 @@ static int selinux_socket_socketpair(struct socket *socka,
>     Need to determine whether we should perform a name_bind
>     permission check between the socket and the port number. */
>
> -static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
> +static int __selinux_socket_bind(struct sock *sk, struct sockaddr *address, int addrlen)
>  {
> -       struct sock *sk = sock->sk;
>         struct sk_security_struct *sksec = selinux_sock(sk);
>         u16 family;
>         int err;
> @@ -5126,13 +5125,17 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>         return -EAFNOSUPPORT;
>  }
>
> +static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
> +{
> +       return __selinux_socket_bind(sock->sk, address, addrlen);
> +}
> +
>  /* This supports connect(2) and SCTP connect services such as sctp_connectx(3)
>   * and sctp_sendmsg(3) as described in Documentation/security/SCTP.rst
>   */
> -static int selinux_socket_connect_helper(struct socket *sock,
> +static int selinux_socket_connect_helper(struct sock *sk,
>                                          struct sockaddr *address, int addrlen)
>  {
> -       struct sock *sk = sock->sk;
>         struct sk_security_struct *sksec = selinux_sock(sk);
>         int err;
>
> @@ -5221,7 +5224,7 @@ static int selinux_socket_connect(struct socket *sock,
>         int err;
>         struct sock *sk = sock->sk;
>
> -       err = selinux_socket_connect_helper(sock, address, addrlen);
> +       err = selinux_socket_connect_helper(sk, address, addrlen);
>         if (err)
>                 return err;
>
> @@ -5706,13 +5709,10 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
>         int len, err = 0, walk_size = 0;
>         void *addr_buf;
>         struct sockaddr *addr;
> -       struct socket *sock;
>
>         if (!selinux_policycap_extsockclass())
>                 return 0;
>
> -       /* Process one or more addresses that may be IPv4 or IPv6 */
> -       sock = sk->sk_socket;

The comment should remain since it wasn't specific to the line of code
you are removing.

>         addr_buf = address;
>
>         while (walk_size < addrlen) {
> @@ -5741,14 +5741,14 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
>                 case SCTP_PRIMARY_ADDR:
>                 case SCTP_SET_PEER_PRIMARY_ADDR:
>                 case SCTP_SOCKOPT_BINDX_ADD:
> -                       err = selinux_socket_bind(sock, addr, len);
> +                       err = __selinux_socket_bind(sk, addr, len);
>                         break;
>                 /* Connect checks */
>                 case SCTP_SOCKOPT_CONNECTX:
>                 case SCTP_PARAM_SET_PRIMARY:
>                 case SCTP_PARAM_ADD_IP:
>                 case SCTP_SENDMSG_CONNECT:
> -                       err = selinux_socket_connect_helper(sock, addr, len);
> +                       err = selinux_socket_connect_helper(sk, addr, len);
>                         if (err)
>                                 return err;
>
> --
> 2.47.3
>