Re: [PATCH net-next] sctp: replace cb->args[4] with a local variable in sctp_diag_dump()

Xin Long <[email protected]> Mon, 13 Jul 2026 15:26:19 -0400
Newsgroups org.kernel.vger.linux-sctp,org.kernel.vger.netdev
Message-ID <CADvbK_e92MPvBRryL_c3D52UtfYKBKjgyH4wb2NGmMDCOAKo1A@mail.gmail.com>
> diff --git a/net/sctp/diag.c b/net/sctp/diag.c
> index c2a0de2adf6f..a9bb31303613 100644
> --- a/net/sctp/diag.c
> +++ b/net/sctp/diag.c
> @@ -299,18 +299,21 @@ static int sctp_sock_dump_one(struct sctp_endpoint *ep, struct sctp_transport *t
>  static int sctp_sock_dump(struct sctp_endpoint *ep, struct sctp_transport *tsp, void *p)
>  {
>       struct sctp_comm_param *commp = p;
> -     struct sock *sk = ep->base.sk;
> +     struct sock *sk = ep->base.sk, *nsk;
>       struct sk_buff *skb = commp->skb;
>       struct netlink_callback *cb = commp->cb;
>       const struct inet_diag_req_v2 *r = commp->r;
> +     u32 portid = NETLINK_CB(cb->skb).portid;
>       struct sctp_association *assoc;
> -     int err = 0;
> +     int err = 0, idx = 0;
> +
> +     nsk = NETLINK_CB(cb->skb).sk;
>
>       lock_sock(sk);
>       if (ep != tsp->asoc->ep)
>               goto release;
Could the peel-off exit here leave stale state in cb->args[1] across
netlink dump sessions?
Prior to this patch, sctp_diag_dump() always executed:
        cb->args[1] = cb->args[4];
        cb->args[4] = 0;
after sctp_transport_traverse_process() returned.  Because cb->args[4]
was only bumped inside the list_for_each_entry loop and was reset to 0
when the loop completed normally, that outer assignment acted as an
unconditional clear of cb->args[1] whenever no traversal was
interrupted.
After the patch, cb->args[1] is only cleared to 0 when the
list_for_each_entry loop in sctp_sock_dump() runs to completion:
>       list_for_each_entry(assoc, &ep->asocs, asocs) {
> -             if (cb->args[4] < cb->args[1])
> +             if (idx < cb->args[1])
>                       goto next;
[ ... ]
>  next:
> -             cb->args[4]++;
> +             idx++;
>       }
>       cb->args[1] = 0;
>       cb->args[3] = 0;
> -     cb->args[4] = 0;
>  release:
>       release_sock(sk);
>       return err;
If a prior session left cb->args[1] = N > 0 and every sctp_sock_dump()
call in the next session hits the early peel-off exit:
        if (ep != tsp->asoc->ep)
                goto release;
then N is never cleared, and a later invocation on an unrelated
endpoint applies "if (idx < cb->args[1]) goto next;" to its ep->asocs,
silently skipping the first N associations.
The trigger requires a previously interrupted dump plus a peel-off race
across every traversed transport in the subsequent session, so the
practical impact is narrow, but should the end-of-traversal path in
sctp_diag_dump() still guarantee cb->args[1] is reset when no
interruption occurred?
[...]

In this case, both cb->args[1] and cb->args[3] need to be cleared.

Please drop this patch, and I will prepare a new patch for net.git.

Thanks.