[PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

__selinux_socket_bind() and selinux_socket_connect_helper() call
sock_has_perm() which uses current_sid() as the AVC subject.  When
selinux_sctp_bind_connect() is invoked from the ASCONF processing
path (sctp_process_asconf), current refers to whichever task was
interrupted, so the permission check evaluates an unrelated subject.

Fix by extracting the sock_has_perm() call out of
__selinux_socket_bind() and selinux_socket_connect_helper() into
their callers.  The process-context wrappers (selinux_socket_bind,
selinux_socket_connect) call sock_has_perm() directly.

For selinux_sctp_bind_connect(), determine the caller SID based on
the optname: SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF
chunk parameter types only passed from the softirq processing path,
so use the socket own SID (sksec->sid), consistent with other
softirq-context hooks such as selinux_socket_sock_rcv_skb() and
selinux_sctp_assoc_request().  All other optnames originate from
process-context syscalls and use current_sid().

Factor out __sock_has_perm() with an explicit subject SID parameter
so that sock_has_perm() remains unchanged for all other callers.

Fixes: d452930fd3b9 ("selinux: Add SCTP support")
Cc: stable@vger.kernel.org
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
Changes in v2:
  - Do not use sksec->sid unconditionally; determine the caller SID
    based on the optname so that process-context callers (bind, connect,
    connectx, sendmsg) still use current_sid()  [Paul Moore, Sashiko]
  - Factor out __sock_has_perm() with an explicit SID parameter and keep
    sock_has_perm() as a thin wrapper for all other callers
  - Move the sock_has_perm() call out of __selinux_socket_bind() and
    selinux_socket_connect_helper() into their respective callers
    (selinux_socket_bind, selinux_socket_connect, selinux_sctp_bind_connect)
 security/selinux/hooks.c | 47 ++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 035aaf113d1da..ab9653ae06fc8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4918,7 +4918,7 @@ static bool sock_skip_has_perm(u32 sid)
 }
 
 
-static int sock_has_perm(struct sock *sk, u32 perms)
+static int __sock_has_perm(struct sock *sk, u32 sid, u32 perms)
 {
 	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct common_audit_data ad;
@@ -4929,10 +4929,15 @@ static int sock_has_perm(struct sock *sk, u32 perms)
 
 	ad_net_init_from_sk(&ad, &net, sk);
 
-	return avc_has_perm(current_sid(), sksec->sid, sksec->sclass, perms,
+	return avc_has_perm(sid, sksec->sid, sksec->sclass, perms,
 			    &ad);
 }
 
+static int sock_has_perm(struct sock *sk, u32 perms)
+{
+	return __sock_has_perm(sk, current_sid(), perms);
+}
+
 static int selinux_socket_create(int family, int type,
 				 int protocol, int kern)
 {
@@ -5006,11 +5011,7 @@ static int __selinux_socket_bind(struct sock *sk, struct sockaddr *address, int
 {
 	struct sk_security_struct *sksec = selinux_sock(sk);
 	u16 family;
-	int err;
-
-	err = sock_has_perm(sk, SOCKET__BIND);
-	if (err)
-		goto out;
+	int err = 0;
 
 	/* If PF_INET or PF_INET6, check name_bind permission for the port. */
 	family = sk->sk_family;
@@ -5135,6 +5136,11 @@ static int __selinux_socket_bind(struct sock *sk, struct sockaddr *address, int
 
 static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
 {
+	int err;
+
+	err = sock_has_perm(sock->sk, SOCKET__BIND);
+	if (err)
+		return err;
 	return __selinux_socket_bind(sock->sk, address, addrlen);
 }
 
@@ -5145,11 +5151,8 @@ static int selinux_socket_connect_helper(struct sock *sk,
 					 struct sockaddr *address, int addrlen)
 {
 	struct sk_security_struct *sksec = selinux_sock(sk);
-	int err;
+	int err = 0;
 
-	err = sock_has_perm(sk, SOCKET__CONNECT);
-	if (err)
-		return err;
 	if (addrlen < offsetofend(struct sockaddr, sa_family))
 		return -EINVAL;
 
@@ -5232,6 +5235,9 @@ static int selinux_socket_connect(struct socket *sock,
 	int err;
 	struct sock *sk = sock->sk;
 
+	err = sock_has_perm(sk, SOCKET__CONNECT);
+	if (err)
+		return err;
 	err = selinux_socket_connect_helper(sk, address, addrlen);
 	if (err)
 		return err;
@@ -5731,13 +5737,25 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
 				     struct sockaddr *address,
 				     int addrlen)
 {
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	int len, err = 0, walk_size = 0;
 	void *addr_buf;
 	struct sockaddr *addr;
+	u32 caller_sid;
 
 	if (!selinux_policycap_extsockclass())
 		return 0;
 
+	/* SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF chunk
+	 * parameter types passed from sctp_process_asconf() in softirq
+	 * context where current is not meaningful. Use the socket own
+	 * SID for permission checks in that case; all other optnames
+	 * originate from process-context syscalls.
+	 */
+	caller_sid = (optname == SCTP_PARAM_SET_PRIMARY ||
+		      optname == SCTP_PARAM_ADD_IP) ?
+		     sksec->sid : current_sid();
+
 	/* Process one or more addresses that may be IPv4 or IPv6 */
 	addr_buf = address;
 
@@ -5767,13 +5785,18 @@ 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(sk, addr, len);
+			err = __sock_has_perm(sk, caller_sid, SOCKET__BIND);
+			if (!err)
+				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 = __sock_has_perm(sk, caller_sid, SOCKET__CONNECT);
+			if (err)
+				return err;
 			err = selinux_socket_connect_helper(sk, addr, len);
 			if (err)
 				return err;
-- 
2.47.3

Re: [PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[sashiko-bot]

> From: Tristan Madani <tristan@talencesecurity.com>
> 
> __selinux_socket_bind() and selinux_socket_connect_helper() call
> sock_has_perm() which uses current_sid() as the AVC subject.  When
> selinux_sctp_bind_connect() is invoked from the ASCONF processing
> path (sctp_process_asconf), current refers to whichever task was
> interrupted, so the permission check evaluates an unrelated subject.
> 
> Fix by extracting the sock_has_perm() call out of
> __selinux_socket_bind() and selinux_socket_connect_helper() into
> their callers.  The process-context wrappers (selinux_socket_bind,
> selinux_socket_connect) call sock_has_perm() directly.
> 
> For selinux_sctp_bind_connect(), determine the caller SID based on
> the optname: SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF
> chunk parameter types only passed from the softirq processing path,
> so use the socket own SID (sksec->sid), consistent with other
> softirq-context hooks such as selinux_socket_sock_rcv_skb() and
> selinux_sctp_assoc_request().  All other optnames originate from
> process-context syscalls and use current_sid().
> 
> Factor out __sock_has_perm() with an explicit subject SID parameter
> so that sock_has_perm() remains unchanged for all other callers.
> 
> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> Cc: stable@vger.kernel.org
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830201157.2084730-1-tristmd@gmail.com?part=1

Re: [PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[Stephen Smalley]

On Sun, Aug 30, 2026 at 4:12 PM Tristan Madani <tristmd@gmail.com> wrote:
>
> From: Tristan Madani <tristan@talencesecurity.com>
>
> __selinux_socket_bind() and selinux_socket_connect_helper() call
> sock_has_perm() which uses current_sid() as the AVC subject.  When
> selinux_sctp_bind_connect() is invoked from the ASCONF processing
> path (sctp_process_asconf), current refers to whichever task was
> interrupted, so the permission check evaluates an unrelated subject.
>
> Fix by extracting the sock_has_perm() call out of
> __selinux_socket_bind() and selinux_socket_connect_helper() into
> their callers.  The process-context wrappers (selinux_socket_bind,
> selinux_socket_connect) call sock_has_perm() directly.
>
> For selinux_sctp_bind_connect(), determine the caller SID based on
> the optname: SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF
> chunk parameter types only passed from the softirq processing path,
> so use the socket own SID (sksec->sid), consistent with other
> softirq-context hooks such as selinux_socket_sock_rcv_skb() and
> selinux_sctp_assoc_request().  All other optnames originate from
> process-context syscalls and use current_sid().
>
> Factor out __sock_has_perm() with an explicit subject SID parameter
> so that sock_has_perm() remains unchanged for all other callers.
>
> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> Cc: stable@vger.kernel.org
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
> Changes in v2:
>   - Do not use sksec->sid unconditionally; determine the caller SID
>     based on the optname so that process-context callers (bind, connect,
>     connectx, sendmsg) still use current_sid()  [Paul Moore, Sashiko]
>   - Factor out __sock_has_perm() with an explicit SID parameter and keep
>     sock_has_perm() as a thin wrapper for all other callers
>   - Move the sock_has_perm() call out of __selinux_socket_bind() and
>     selinux_socket_connect_helper() into their respective callers
>     (selinux_socket_bind, selinux_socket_connect, selinux_sctp_bind_connect)
>  security/selinux/hooks.c | 47 ++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 035aaf113d1da..ab9653ae06fc8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4918,7 +4918,7 @@ static bool sock_skip_has_perm(u32 sid)
>  }
>
>
> -static int sock_has_perm(struct sock *sk, u32 perms)
> +static int __sock_has_perm(struct sock *sk, u32 sid, u32 perms)
>  {
>         struct sk_security_struct *sksec = selinux_sock(sk);
>         struct common_audit_data ad;
> @@ -4929,10 +4929,15 @@ static int sock_has_perm(struct sock *sk, u32 perms)
>
>         ad_net_init_from_sk(&ad, &net, sk);
>
> -       return avc_has_perm(current_sid(), sksec->sid, sksec->sclass, perms,
> +       return avc_has_perm(sid, sksec->sid, sksec->sclass, perms,
>                             &ad);
>  }
>
> +static int sock_has_perm(struct sock *sk, u32 perms)
> +{
> +       return __sock_has_perm(sk, current_sid(), perms);
> +}
> +
>  static int selinux_socket_create(int family, int type,
>                                  int protocol, int kern)
>  {
> @@ -5006,11 +5011,7 @@ static int __selinux_socket_bind(struct sock *sk, struct sockaddr *address, int
>  {
>         struct sk_security_struct *sksec = selinux_sock(sk);
>         u16 family;
> -       int err;
> -
> -       err = sock_has_perm(sk, SOCKET__BIND);
> -       if (err)
> -               goto out;
> +       int err = 0;
>
>         /* If PF_INET or PF_INET6, check name_bind permission for the port. */
>         family = sk->sk_family;
> @@ -5135,6 +5136,11 @@ static int __selinux_socket_bind(struct sock *sk, struct sockaddr *address, int
>
>  static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
>  {
> +       int err;
> +
> +       err = sock_has_perm(sock->sk, SOCKET__BIND);
> +       if (err)
> +               return err;
>         return __selinux_socket_bind(sock->sk, address, addrlen);
>  }
>
> @@ -5145,11 +5151,8 @@ static int selinux_socket_connect_helper(struct sock *sk,
>                                          struct sockaddr *address, int addrlen)
>  {
>         struct sk_security_struct *sksec = selinux_sock(sk);
> -       int err;
> +       int err = 0;
>
> -       err = sock_has_perm(sk, SOCKET__CONNECT);
> -       if (err)
> -               return err;
>         if (addrlen < offsetofend(struct sockaddr, sa_family))
>                 return -EINVAL;
>
> @@ -5232,6 +5235,9 @@ static int selinux_socket_connect(struct socket *sock,
>         int err;
>         struct sock *sk = sock->sk;
>
> +       err = sock_has_perm(sk, SOCKET__CONNECT);
> +       if (err)
> +               return err;
>         err = selinux_socket_connect_helper(sk, address, addrlen);
>         if (err)
>                 return err;
> @@ -5731,13 +5737,25 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
>                                      struct sockaddr *address,
>                                      int addrlen)
>  {
> +       struct sk_security_struct *sksec = selinux_sock(sk);
>         int len, err = 0, walk_size = 0;
>         void *addr_buf;
>         struct sockaddr *addr;
> +       u32 caller_sid;
>
>         if (!selinux_policycap_extsockclass())
>                 return 0;
>
> +       /* SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF chunk
> +        * parameter types passed from sctp_process_asconf() in softirq
> +        * context where current is not meaningful. Use the socket own
> +        * SID for permission checks in that case; all other optnames
> +        * originate from process-context syscalls.
> +        */
> +       caller_sid = (optname == SCTP_PARAM_SET_PRIMARY ||
> +                     optname == SCTP_PARAM_ADD_IP) ?
> +                    sksec->sid : current_sid();
> +
>         /* Process one or more addresses that may be IPv4 or IPv6 */
>         addr_buf = address;
>
> @@ -5767,13 +5785,18 @@ 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(sk, addr, len);
> +                       err = __sock_has_perm(sk, caller_sid, SOCKET__BIND);
> +                       if (!err)
> +                               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 = __sock_has_perm(sk, caller_sid, SOCKET__CONNECT);
> +                       if (err)
> +                               return err;
>                         err = selinux_socket_connect_helper(sk, addr, len);
>                         if (err)
>                                 return err;
> --
> 2.47.3
>

Re: [PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[Paul Moore]

On Aug 30, 2026 Tristan Madani <tristmd@gmail.com> wrote:
> 
> __selinux_socket_bind() and selinux_socket_connect_helper() call
> sock_has_perm() which uses current_sid() as the AVC subject.  When
> selinux_sctp_bind_connect() is invoked from the ASCONF processing
> path (sctp_process_asconf), current refers to whichever task was
> interrupted, so the permission check evaluates an unrelated subject.
> 
> Fix by extracting the sock_has_perm() call out of
> __selinux_socket_bind() and selinux_socket_connect_helper() into
> their callers.  The process-context wrappers (selinux_socket_bind,
> selinux_socket_connect) call sock_has_perm() directly.
> 
> For selinux_sctp_bind_connect(), determine the caller SID based on
> the optname: SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF
> chunk parameter types only passed from the softirq processing path,
> so use the socket own SID (sksec->sid), consistent with other
> softirq-context hooks such as selinux_socket_sock_rcv_skb() and
> selinux_sctp_assoc_request().  All other optnames originate from
> process-context syscalls and use current_sid().
> 
> Factor out __sock_has_perm() with an explicit subject SID parameter
> so that sock_has_perm() remains unchanged for all other callers.
> 
> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> Cc: stable@vger.kernel.org
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> Changes in v2:
>   - Do not use sksec->sid unconditionally; determine the caller SID
>     based on the optname so that process-context callers (bind, connect,
>     connectx, sendmsg) still use current_sid()  [Paul Moore, Sashiko]
>   - Factor out __sock_has_perm() with an explicit SID parameter and keep
>     sock_has_perm() as a thin wrapper for all other callers
>   - Move the sock_has_perm() call out of __selinux_socket_bind() and
>     selinux_socket_connect_helper() into their respective callers
>     (selinux_socket_bind, selinux_socket_connect, selinux_sctp_bind_connect)
>  security/selinux/hooks.c | 47 ++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 12 deletions(-)

Hi Tristan, this looks much better thank you for putting together this
revised patch.  Unfortunately, while looking at it I did notice one thing
(below) which I think we need to change ...

> @@ -5731,13 +5737,25 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
>  				     struct sockaddr *address,
>  				     int addrlen)
>  {
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>  	int len, err = 0, walk_size = 0;
>  	void *addr_buf;
>  	struct sockaddr *addr;
> +	u32 caller_sid;
>  
>  	if (!selinux_policycap_extsockclass())
>  		return 0;
>  
> +	/* SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF chunk
> +	 * parameter types passed from sctp_process_asconf() in softirq
> +	 * context where current is not meaningful. Use the socket own
> +	 * SID for permission checks in that case; all other optnames
> +	 * originate from process-context syscalls.
> +	 */
> +	caller_sid = (optname == SCTP_PARAM_SET_PRIMARY ||
> +		      optname == SCTP_PARAM_ADD_IP) ?
> +		     sksec->sid : current_sid();

This should have caught my eye in the previous iteration, I'm sorry about
that, but now that I'm looking at this it occurs to me that the sock's
label isn't the subject in the ASCONF case, it's the label of the remote
node (sksec->peer_sid).  After all, the ASCONF operation doesn't originate
with the local socket, as would be implied by sksec->sid, it comes from the
remote endpoint which is represented by sksec->peer_sid.

>  	/* Process one or more addresses that may be IPv4 or IPv6 */
>  	addr_buf = address;
>  
> @@ -5767,13 +5785,18 @@ 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(sk, addr, len);
> +			err = __sock_has_perm(sk, caller_sid, SOCKET__BIND);
> +			if (!err)
> +				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 = __sock_has_perm(sk, caller_sid, SOCKET__CONNECT);
> +			if (err)
> +				return err;
>  			err = selinux_socket_connect_helper(sk, addr, len);
>  			if (err)
>  				return err;
> -- 
> 2.47.3

--
paul-moore.com

Re: [PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[Stephen Smalley]

On Wed, Sep 2, 2026 at 10:24 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Aug 30, 2026 Tristan Madani <tristmd@gmail.com> wrote:
> >
> > __selinux_socket_bind() and selinux_socket_connect_helper() call
> > sock_has_perm() which uses current_sid() as the AVC subject.  When
> > selinux_sctp_bind_connect() is invoked from the ASCONF processing
> > path (sctp_process_asconf), current refers to whichever task was
> > interrupted, so the permission check evaluates an unrelated subject.
> >
> > Fix by extracting the sock_has_perm() call out of
> > __selinux_socket_bind() and selinux_socket_connect_helper() into
> > their callers.  The process-context wrappers (selinux_socket_bind,
> > selinux_socket_connect) call sock_has_perm() directly.
> >
> > For selinux_sctp_bind_connect(), determine the caller SID based on
> > the optname: SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF
> > chunk parameter types only passed from the softirq processing path,
> > so use the socket own SID (sksec->sid), consistent with other
> > softirq-context hooks such as selinux_socket_sock_rcv_skb() and
> > selinux_sctp_assoc_request().  All other optnames originate from
> > process-context syscalls and use current_sid().
> >
> > Factor out __sock_has_perm() with an explicit subject SID parameter
> > so that sock_has_perm() remains unchanged for all other callers.
> >
> > Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> > Cc: stable@vger.kernel.org
> > Suggested-by: Paul Moore <paul@paul-moore.com>
> > Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> > Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> > ---
> > Changes in v2:
> >   - Do not use sksec->sid unconditionally; determine the caller SID
> >     based on the optname so that process-context callers (bind, connect,
> >     connectx, sendmsg) still use current_sid()  [Paul Moore, Sashiko]
> >   - Factor out __sock_has_perm() with an explicit SID parameter and keep
> >     sock_has_perm() as a thin wrapper for all other callers
> >   - Move the sock_has_perm() call out of __selinux_socket_bind() and
> >     selinux_socket_connect_helper() into their respective callers
> >     (selinux_socket_bind, selinux_socket_connect, selinux_sctp_bind_connect)
> >  security/selinux/hooks.c | 47 ++++++++++++++++++++++++++++++----------
> >  1 file changed, 35 insertions(+), 12 deletions(-)
>
> Hi Tristan, this looks much better thank you for putting together this
> revised patch.  Unfortunately, while looking at it I did notice one thing
> (below) which I think we need to change ...
>
> > @@ -5731,13 +5737,25 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
> >                                    struct sockaddr *address,
> >                                    int addrlen)
> >  {
> > +     struct sk_security_struct *sksec = selinux_sock(sk);
> >       int len, err = 0, walk_size = 0;
> >       void *addr_buf;
> >       struct sockaddr *addr;
> > +     u32 caller_sid;
> >
> >       if (!selinux_policycap_extsockclass())
> >               return 0;
> >
> > +     /* SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF chunk
> > +      * parameter types passed from sctp_process_asconf() in softirq
> > +      * context where current is not meaningful. Use the socket own
> > +      * SID for permission checks in that case; all other optnames
> > +      * originate from process-context syscalls.
> > +      */
> > +     caller_sid = (optname == SCTP_PARAM_SET_PRIMARY ||
> > +                   optname == SCTP_PARAM_ADD_IP) ?
> > +                  sksec->sid : current_sid();
>
> This should have caught my eye in the previous iteration, I'm sorry about
> that, but now that I'm looking at this it occurs to me that the sock's
> label isn't the subject in the ASCONF case, it's the label of the remote
> node (sksec->peer_sid).  After all, the ASCONF operation doesn't originate
> with the local socket, as would be implied by sksec->sid, it comes from the
> remote endpoint which is represented by sksec->peer_sid.

Are you sure that's the behavior you want? I don't think there is
precedent for performing the socket layer checks with the peer SID as
the subject rather than the local socket SID, and if using NetLabel,
that peer SID will be limited to the peer's MLS label and whatever
type was assigned via netlabelctl.

Re: [PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[Paul Moore]

On Thu, Sep 3, 2026 at 8:32 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Wed, Sep 2, 2026 at 10:24 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Aug 30, 2026 Tristan Madani <tristmd@gmail.com> wrote:
> > >
> > > __selinux_socket_bind() and selinux_socket_connect_helper() call
> > > sock_has_perm() which uses current_sid() as the AVC subject.  When
> > > selinux_sctp_bind_connect() is invoked from the ASCONF processing
> > > path (sctp_process_asconf), current refers to whichever task was
> > > interrupted, so the permission check evaluates an unrelated subject.
> > >
> > > Fix by extracting the sock_has_perm() call out of
> > > __selinux_socket_bind() and selinux_socket_connect_helper() into
> > > their callers.  The process-context wrappers (selinux_socket_bind,
> > > selinux_socket_connect) call sock_has_perm() directly.
> > >
> > > For selinux_sctp_bind_connect(), determine the caller SID based on
> > > the optname: SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF
> > > chunk parameter types only passed from the softirq processing path,
> > > so use the socket own SID (sksec->sid), consistent with other
> > > softirq-context hooks such as selinux_socket_sock_rcv_skb() and
> > > selinux_sctp_assoc_request().  All other optnames originate from
> > > process-context syscalls and use current_sid().
> > >
> > > Factor out __sock_has_perm() with an explicit subject SID parameter
> > > so that sock_has_perm() remains unchanged for all other callers.
> > >
> > > Fixes: d452930fd3b9 ("selinux: Add SCTP support")
> > > Cc: stable@vger.kernel.org
> > > Suggested-by: Paul Moore <paul@paul-moore.com>
> > > Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> > > Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> > > ---
> > > Changes in v2:
> > >   - Do not use sksec->sid unconditionally; determine the caller SID
> > >     based on the optname so that process-context callers (bind, connect,
> > >     connectx, sendmsg) still use current_sid()  [Paul Moore, Sashiko]
> > >   - Factor out __sock_has_perm() with an explicit SID parameter and keep
> > >     sock_has_perm() as a thin wrapper for all other callers
> > >   - Move the sock_has_perm() call out of __selinux_socket_bind() and
> > >     selinux_socket_connect_helper() into their respective callers
> > >     (selinux_socket_bind, selinux_socket_connect, selinux_sctp_bind_connect)
> > >  security/selinux/hooks.c | 47 ++++++++++++++++++++++++++++++----------
> > >  1 file changed, 35 insertions(+), 12 deletions(-)
> >
> > Hi Tristan, this looks much better thank you for putting together this
> > revised patch.  Unfortunately, while looking at it I did notice one thing
> > (below) which I think we need to change ...
> >
> > > @@ -5731,13 +5737,25 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
> > >                                    struct sockaddr *address,
> > >                                    int addrlen)
> > >  {
> > > +     struct sk_security_struct *sksec = selinux_sock(sk);
> > >       int len, err = 0, walk_size = 0;
> > >       void *addr_buf;
> > >       struct sockaddr *addr;
> > > +     u32 caller_sid;
> > >
> > >       if (!selinux_policycap_extsockclass())
> > >               return 0;
> > >
> > > +     /* SCTP_PARAM_SET_PRIMARY and SCTP_PARAM_ADD_IP are ASCONF chunk
> > > +      * parameter types passed from sctp_process_asconf() in softirq
> > > +      * context where current is not meaningful. Use the socket own
> > > +      * SID for permission checks in that case; all other optnames
> > > +      * originate from process-context syscalls.
> > > +      */
> > > +     caller_sid = (optname == SCTP_PARAM_SET_PRIMARY ||
> > > +                   optname == SCTP_PARAM_ADD_IP) ?
> > > +                  sksec->sid : current_sid();
> >
> > This should have caught my eye in the previous iteration, I'm sorry about
> > that, but now that I'm looking at this it occurs to me that the sock's
> > label isn't the subject in the ASCONF case, it's the label of the remote
> > node (sksec->peer_sid).  After all, the ASCONF operation doesn't originate
> > with the local socket, as would be implied by sksec->sid, it comes from the
> > remote endpoint which is represented by sksec->peer_sid.
>
> Are you sure that's the behavior you want?

Think about how the socket/connect check works for local operations:
the task requesting the socket/connect operation is the subject and
the socket is the object.  In the currently proposed patch, when an
ASCONF socket/connect operation is requested, the subject is treated
completely differently as the socket is used to represent both the
subject and object.  Even if we consider the socket's label a proxy
for the creating task, using the socket's label as the subject of an
ASCONF permission check makes little sense as it is the remote
endpoint that triggers the ASCONF operation, not a local entity.

> I don't think there is
> precedent for performing the socket layer checks with the peer SID as
> the subject rather than the local socket SID ...

I think that is because all (?) of the other socket layer checks are
initiated by the local system.  The ASCONF operations are unique
because a remote node initiates them, which is why the network peer
label is used as the subject as that best represents the remote
endpoint.

Please feel free to put forth your own reasoning on this, a discussion
would be good, but I think we do need to recognize that ASCONF
operations are different from local socket operations.

> ... and if using NetLabel,
> that peer SID will be limited to the peer's MLS label and whatever
> type was assigned via netlabelctl.

Limitations in the peer label don't change that it is the remote note
triggering the operation and not a local task.

-- 
paul-moore.com

Re: [PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[Stephen Smalley]

On Thu, Sep 3, 2026 at 11:48 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Sep 3, 2026 at 8:32 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > Are you sure that's the behavior you want?
>
> Think about how the socket/connect check works for local operations:
> the task requesting the socket/connect operation is the subject and
> the socket is the object.  In the currently proposed patch, when an
> ASCONF socket/connect operation is requested, the subject is treated
> completely differently as the socket is used to represent both the
> subject and object.  Even if we consider the socket's label a proxy
> for the creating task, using the socket's label as the subject of an
> ASCONF permission check makes little sense as it is the remote
> endpoint that triggers the ASCONF operation, not a local entity.
>
> > I don't think there is
> > precedent for performing the socket layer checks with the peer SID as
> > the subject rather than the local socket SID ...
>
> I think that is because all (?) of the other socket layer checks are
> initiated by the local system.  The ASCONF operations are unique
> because a remote node initiates them, which is why the network peer
> label is used as the subject as that best represents the remote
> endpoint.
>
> Please feel free to put forth your own reasoning on this, a discussion
> would be good, but I think we do need to recognize that ASCONF
> operations are different from local socket operations.

That seems reasonable. Wondering if this change will then require
updates to the selinux-testsuite (or if not, we ought to test this
behavior too).

Re: [PATCH v2] selinux: use socket SID for SCTP ASCONF permission checks

[Paul Moore]

On Fri, Sep 4, 2026 at 8:11 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Thu, Sep 3, 2026 at 11:48 AM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Thu, Sep 3, 2026 at 8:32 AM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > > Are you sure that's the behavior you want?
> >
> > Think about how the socket/connect check works for local operations:
> > the task requesting the socket/connect operation is the subject and
> > the socket is the object.  In the currently proposed patch, when an
> > ASCONF socket/connect operation is requested, the subject is treated
> > completely differently as the socket is used to represent both the
> > subject and object.  Even if we consider the socket's label a proxy
> > for the creating task, using the socket's label as the subject of an
> > ASCONF permission check makes little sense as it is the remote
> > endpoint that triggers the ASCONF operation, not a local entity.
> >
> > > I don't think there is
> > > precedent for performing the socket layer checks with the peer SID as
> > > the subject rather than the local socket SID ...
> >
> > I think that is because all (?) of the other socket layer checks are
> > initiated by the local system.  The ASCONF operations are unique
> > because a remote node initiates them, which is why the network peer
> > label is used as the subject as that best represents the remote
> > endpoint.
> >
> > Please feel free to put forth your own reasoning on this, a discussion
> > would be good, but I think we do need to recognize that ASCONF
> > operations are different from local socket operations.
>
> That seems reasonable. Wondering if this change will then require
> updates to the selinux-testsuite (or if not, we ought to test this
> behavior too).

Yes, definitely.

-- 
paul-moore.com