Re: [PATCH net-next] sctp: replace cb->args[4] with a local variable in sctp_diag_dump()
From: Xin Long <lucien.xin@gmail.com>
Date: 2026-07-13 19:26:30
Also in:
linux-sctp
quoted hunk ↗ jump to hunk
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.