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.