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

[Paul Moore <paul@paul-moore.com>]

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