[PATCH] selinux: fix NULL pointer dereference in selinux_sctp_bind_connect()

[PATCH] selinux: fix NULL pointer dereference in selinux_sctp_bind_connect()

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

selinux_sctp_bind_connect() reads sk->sk_socket and passes it to
selinux_socket_bind() or selinux_socket_connect_helper() without
checking for NULL.  When an SCTP ASCONF chunk is processed in softirq
context on a socket that has been concurrently closed, sock_orphan()
will have already set sk->sk_socket to NULL.  The subsequent
dereference of sock->sk at offset 0x18 triggers a kernel panic.

Add a NULL check on sk->sk_socket before use.

Fixes: d452930fd3b9 ("selinux: Add SCTP support")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 security/selinux/hooks.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0f704380a8c8..e45588563caa 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5717,6 +5717,9 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
 
 	/* Process one or more addresses that may be IPv4 or IPv6 */
 	sock = sk->sk_socket;
+	if (!sock)
+		return -ECONNRESET;
+
 	addr_buf = address;
 
 	while (walk_size < addrlen) {
-- 
2.47.3

Re: [PATCH] selinux: fix NULL pointer dereference in selinux_sctp_bind_connect()

[Stephen Smalley]

On Thu, Jun 18, 2026 at 7:21 PM Tristan Madani <tristmd@gmail.com> wrote:
>
> From: Tristan Madani <tristan@talencesecurity.com>
>
> selinux_sctp_bind_connect() reads sk->sk_socket and passes it to
> selinux_socket_bind() or selinux_socket_connect_helper() without
> checking for NULL.  When an SCTP ASCONF chunk is processed in softirq
> context on a socket that has been concurrently closed, sock_orphan()
> will have already set sk->sk_socket to NULL.  The subsequent
> dereference of sock->sk at offset 0x18 triggers a kernel panic.
>
> Add a NULL check on sk->sk_socket before use.

Is this sufficient, or can the sk_socket be freed under us after the assignment?
Do different callers of this hook provide different guarantees
regarding sk_socket or are they all the same?

>
> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  security/selinux/hooks.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0f704380a8c8..e45588563caa 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5717,6 +5717,9 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
>
>         /* Process one or more addresses that may be IPv4 or IPv6 */
>         sock = sk->sk_socket;
> +       if (!sock)
> +               return -ECONNRESET;
> +
>         addr_buf = address;
>
>         while (walk_size < addrlen) {
> --
> 2.47.3
>

Re: [PATCH] selinux: fix NULL pointer dereference in selinux_sctp_bind_connect()

[Tristan Madani]

On 2026/06/22 10:12, Stephen Smalley wrote:
> Is this sufficient, or can the sk_socket be freed under us after the
> assignment?

The assignment is safe. sock_orphan() only NULLs sk->sk_socket -- the
struct socket is freed later in __sock_release(), after inet_release()
returns. That path goes through sctp_close() -> lock_sock(), which
serializes with the ASCONF softirq path (bh_lock_sock). So once we
read a non-NULL pointer into the local variable, the socket is
guaranteed to remain alive for the duration of the function.

> Do different callers of this hook provide different guarantees
> regarding sk_socket or are they all the same?

They differ. The setsockopt callers (bindx, connectx, set_primary,
sendmsg connect) run in process context with a file reference, so
sk_socket is guaranteed non-NULL. The ASCONF softirq path
(sctp_process_asconf) has no file reference and can race with socket
close -- that is the only caller that can hit the NULL.

Tristan

Re: [PATCH] selinux: fix NULL pointer dereference in selinux_sctp_bind_connect()

[Stephen Smalley]

On Mon, Jun 22, 2026 at 2:59 PM Tristan Madani <tristmd@gmail.com> wrote:
>
> On 2026/06/22 10:12, Stephen Smalley wrote:
> > Is this sufficient, or can the sk_socket be freed under us after the
> > assignment?
>
> The assignment is safe. sock_orphan() only NULLs sk->sk_socket -- the
> struct socket is freed later in __sock_release(), after inet_release()
> returns. That path goes through sctp_close() -> lock_sock(), which
> serializes with the ASCONF softirq path (bh_lock_sock). So once we
> read a non-NULL pointer into the local variable, the socket is
> guaranteed to remain alive for the duration of the function.
>
> > Do different callers of this hook provide different guarantees
> > regarding sk_socket or are they all the same?
>
> They differ. The setsockopt callers (bindx, connectx, set_primary,
> sendmsg connect) run in process context with a file reference, so
> sk_socket is guaranteed non-NULL. The ASCONF softirq path
> (sctp_process_asconf) has no file reference and can race with socket
> close -- that is the only caller that can hit the NULL.

Thank you for clarifying. It might be good to add some or all of the
above to the patch description and/or
a comment in the code to make it clear going forward.

Re: [PATCH] selinux: fix NULL pointer dereference in selinux_sctp_bind_connect()

[Stephen Smalley]

On Mon, Jun 22, 2026 at 3:15 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Jun 22, 2026 at 2:59 PM Tristan Madani <tristmd@gmail.com> wrote:
> >
> > On 2026/06/22 10:12, Stephen Smalley wrote:
> > > Is this sufficient, or can the sk_socket be freed under us after the
> > > assignment?
> >
> > The assignment is safe. sock_orphan() only NULLs sk->sk_socket -- the
> > struct socket is freed later in __sock_release(), after inet_release()
> > returns. That path goes through sctp_close() -> lock_sock(), which
> > serializes with the ASCONF softirq path (bh_lock_sock). So once we
> > read a non-NULL pointer into the local variable, the socket is
> > guaranteed to remain alive for the duration of the function.
> >
> > > Do different callers of this hook provide different guarantees
> > > regarding sk_socket or are they all the same?
> >
> > They differ. The setsockopt callers (bindx, connectx, set_primary,
> > sendmsg connect) run in process context with a file reference, so
> > sk_socket is guaranteed non-NULL. The ASCONF softirq path
> > (sctp_process_asconf) has no file reference and can race with socket
> > close -- that is the only caller that can hit the NULL.
>
> Thank you for clarifying. It might be good to add some or all of the
> above to the patch description and/or
> a comment in the code to make it clear going forward.

Actually, given that selinux_sctp_bind_connect() just passes the
socket to selinux_socket_bind() or selinux_socket_connect_helper() and
the first thing those functions do is to grab the sock from it, it
seems like we could just refactor helpers that directly take the
struct sock * and never have to deal with this issue at all.

Otherwise, we likely want a READ_ONCE() here as well.

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

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

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@gmail.com>
Fixes: d452930fd3b9 ("selinux: Add SCTP support")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
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;
 	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

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@gmail.com> wrote:
>
> From: Tristan Madani <tristan@talencesecurity.com>
>
> 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@gmail.com>
> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> 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
>