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@xxxxxxxxx> wrote:
>
> On Mon, Jun 22, 2026 at 2:59 PM Tristan Madani <tristmd@xxxxxxxxx> 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.