Re: [PATCH net 1/1] sctp: diag: reject stale associations in dump_one path

Xin Long <[email protected]> Mon, 1 Jun 2026 13:43:34 -0400
Newsgroups org.kernel.vger.linux-sctp,org.kernel.vger.netdev
Message-ID <CADvbK_cXhGqgEua975KuP+Vu81s7ZjYjE=kTiMrwxcBKLdb0VA@mail.gmail.com>
On Sat, May 30, 2026 at 11:57 AM Ren Wei <[email protected]> wrote:
>
> From: Zhao Zhang <[email protected]>
>
> The SCTP exact sock_diag lookup can hold a transport reference, block on
> lock_sock(sk), and then resume after sctp_association_free() has marked
> the association dead and freed its bind address list.
>
> When that happens, inet_assoc_attr_size() and
> inet_diag_msg_sctpasoc_fill() can still dereference association state
> that is no longer valid for reporting. In particular,
> inet_diag_msg_sctpasoc_fill() may read an empty bind-address list as a
> real sctp_sockaddr_entry and trigger an out-of-bounds read from
> unrelated association memory.
>
> Reject the association after taking the socket lock if it has been
> reaped or detached from the endpoint, and report the lookup as stale.
> This keeps the exact dump-one path from formatting torn association
> state.
>
> Fixes: 8f840e47f190 ("sctp: add the sctp_diag.c file")
> Cc: [email protected]
> Reported-by: Yuan Tan <[email protected]>
> Reported-by: Yifan Wu <[email protected]>
> Reported-by: Juefei Pu <[email protected]>
> Reported-by: Zhengchuan Liang <[email protected]>
> Reported-by: Xin Liu <[email protected]>
> Assisted-by: Codex:GPT-5.4
> Signed-off-by: Zhao Zhang <[email protected]>
> Signed-off-by: Ren Wei <[email protected]>
> ---
>  net/sctp/diag.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/net/sctp/diag.c b/net/sctp/diag.c
> index 2afb376299fe..d758f5c3e06e 100644
> --- a/net/sctp/diag.c
> +++ b/net/sctp/diag.c
> @@ -266,15 +266,15 @@ static int sctp_sock_dump_one(struct sctp_endpoint *ep, struct sctp_transport *t
>
>         lock_sock(sk);
>
> -       rep = nlmsg_new(inet_assoc_attr_size(sk, assoc), GFP_KERNEL);
> -       if (!rep) {
> -               release_sock(sk);
> -               return -ENOMEM;
> +       if (ep != assoc->ep || assoc->base.dead) {
> +               err = -ESTALE;
> +               goto out_unlock;
>         }
>
> -       if (ep != assoc->ep) {
> -               err = -EAGAIN;
> -               goto out;
> +       rep = nlmsg_new(inet_assoc_attr_size(sk, assoc), GFP_KERNEL);
> +       if (!rep) {
> +               err = -ENOMEM;
> +               goto out_unlock;
>         }
>
>         err = inet_sctp_diag_fill(sk, assoc, rep, req, sk_user_ns(NETLINK_CB(skb).sk),
> @@ -289,8 +289,9 @@ static int sctp_sock_dump_one(struct sctp_endpoint *ep, struct sctp_transport *t
>         return nlmsg_unicast(sock_net(skb->sk)->diag_nlsk, rep, NETLINK_CB(skb).portid);
>
>  out:
> -       release_sock(sk);
>         kfree_skb(rep);
> +out_unlock:
> +       release_sock(sk);
>         return err;
>  }
>
> --
> 2.47.3
>
Thanks for the fix.

Acked-by: Xin Long <[email protected]>

Note that the issue reported in
https://sashiko.dev/#/patchset/fac6043fa20a2ff68e12958c431836f692c51268.1780113823.git.zzhan461%40ucr.edu.

I don't think it exists, as sctp_sock_filter() is called via
sctp_transport_traverse_process() where sctp_transport_get_next() only
returns primary_path's transport, not each transport.