[PATCH net v2] sctp: fix stream->outcnt underflow on duplicate RECONF responses
From: Jun Yang <hidden>
Date: 2026-08-04 11:31:13
Also in:
linux-sctp, lkml
Subsystem:
networking [general], sctp protocol, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marcelo Ricardo Leitner, Xin Long, Linus Torvalds
From: Jun Yang <redacted>
sctp_process_strreset_resp() rolls back a denied ADD_OUT_STREAMS request
by subtracting the requested count from the current stream count:
nums = ntohs(addstrm->number_of_streams);
number = stream->outcnt - nums; /* net/sctp/stream.c:1050 */
...
stream->outcnt = number; /* net/sctp/stream.c:1060 */
This undoes the increment sctp_send_add_streams() performed at request
time, and is only correct if it runs exactly once per request. Nothing
enforces that.
asoc->strreset_outstanding is a plain count of the request parameters on
the fly, so it cannot tell which request a response belongs to, and
sctp_chunk_lookup_strreset_param() accepts any parameter still present
in asoc->strreset_chunk, which stays live until that count reaches zero.
When both outgoing and incoming streams are added in one
setsockopt(SCTP_ADD_STREAMS), sctp_send_add_streams() sets the count to
2 and caches a chunk holding both the ADD_OUT and ADD_IN parameters.
sctp_verify_reconf() permits a RESET_RESPONSE to follow another
RESET_RESPONSE, so a peer can put two responses carrying the ADD_OUT
request_seq into a single RECONF chunk. sctp_sf_do_reconf() processes
both: the first rollback restores the original count and the second
subtracts nums again. Depending on the counts, this either wraps the
__u16 or silently shrinks the stream count a second time.
SCTP_SO() is genradix_ptr(), which returns NULL past the preallocated
range, so sctp_sendmsg_to_asoc() accepts an out-of-range stream id at
net/sctp/socket.c:1803 and dereferences the resulting NULL slot at
net/sctp/socket.c:1808. Other stream walkers likewise trust the
inflated count until a later operation repairs or tears down the
association.
Turn strreset_outstanding into a bitmask of the request parameter types
on the fly, one bit per SCTP_PARAM_RESET_* request type, and process a
response only if its request type is still outstanding. Handling a
response clears its bit, so a duplicate is dropped, while responses for
the other parameters of the same RECONF chunk are still accepted in any
order.
Test the bit in sctp_process_strreset_outreq() and
sctp_process_strreset_addstrm_out() as well: a peer request that
implicitly answers our IN/ADD_IN request could otherwise answer it
twice, retiring the cached chunk and its reconf timer while another
request was still waiting for a response.
Fixes: 11ae76e67a17 ("sctp: implement receiver-side procedures for the Reconf Response Parameter")
Cc: stable@kernel.org
Reported-by: TencentOS Corvus AI <redacted>
Suggested-by: Xin Long <lucien.xin@gmail.com>
Assisted-by: tencentos-corvus-ai:kimi-k3
Signed-off-by: Jun Yang <redacted>
---
A KASAN reproducer for this issue is available if requested.
v2:
- Drop the "resp->response_seq != htonl(asoc->strreset_outseq)" test: an
ADD_IN/IN request uses outseq + 1, so it rejected a valid response
that arrives before the ADD_OUT/OUT one. Drop the
!strreset_outstanding test as well, it does not stop a duplicate
while another parameter of the same chunk is still on the fly
(Xin Long).
- Instead track the requests on the fly as a per request type bitmask
and test it in sctp_process_strreset_resp(), so a duplicate is
rejected per request type, regardless of the arrival order.
- Drop the "nums > stream->outcnt" test, no longer needed once the
duplicate response is rejected (Xin Long).
v1: https://lore.kernel.org/netdev/20260730110225.37371-1-juny24602@gmail.com/ (local)
include/net/sctp/structs.h | 16 +++++++++++++++-
net/sctp/stream.c | 30 +++++++++++++++++++-----------
2 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index cccc662561aa..0b48c45d647e 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h@@ -2057,7 +2057,7 @@ struct sctp_association { force_delay:1; __u8 strreset_enable; - __u8 strreset_outstanding; /* request param count on the fly */ + __u8 strreset_outstanding; /* request param bitmask on the fly */ __u32 strreset_outseq; /* Update after receiving response */ __u32 strreset_inseq; /* Update after receiving request */
@@ -2087,6 +2087,20 @@ struct sctp_association { struct rcu_head rcu; }; +/* Track the outstanding stream reconf requests per request param type, so + * that a response can only clear the request it belongs to, and only once. + * The reconf param types range from SCTP_PARAM_RESET_OUT_REQUEST (0x000d) + * to SCTP_PARAM_RESET_ADD_IN_STREAMS (0x0012), so one bit each fits in + * asoc->strreset_outstanding. + */ +#define SCTP_STRRESET_MASK(type) \ + (1 << (ntohs(type) - ntohs(SCTP_PARAM_RESET_OUT_REQUEST))) +#define SCTP_STRRESET_TEST(asoc, type) \ + ((asoc)->strreset_outstanding & SCTP_STRRESET_MASK(type)) +#define SCTP_STRRESET_SET(asoc, type) \ + ((asoc)->strreset_outstanding |= SCTP_STRRESET_MASK(type)) +#define SCTP_STRRESET_CLEAR(asoc, type) \ + ((asoc)->strreset_outstanding &= ~SCTP_STRRESET_MASK(type)) /* An eyecatcher for determining if we are really looking at an * association data structure.
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 34ffe6c945a4..cb7543929d65 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c@@ -372,7 +372,10 @@ int sctp_send_reset_streams(struct sctp_association *asoc, goto out; } - asoc->strreset_outstanding = out + in; + if (out) + SCTP_STRRESET_SET(asoc, SCTP_PARAM_RESET_OUT_REQUEST); + if (in) + SCTP_STRRESET_SET(asoc, SCTP_PARAM_RESET_IN_REQUEST); out: return retval;
@@ -417,7 +420,7 @@ int sctp_send_reset_assoc(struct sctp_association *asoc) return retval; } - asoc->strreset_outstanding = 1; + SCTP_STRRESET_SET(asoc, SCTP_PARAM_RESET_TSN_REQUEST); return 0; }
@@ -474,7 +477,10 @@ int sctp_send_add_streams(struct sctp_association *asoc, goto out; } - asoc->strreset_outstanding = !!out + !!in; + if (out) + SCTP_STRRESET_SET(asoc, SCTP_PARAM_RESET_ADD_OUT_STREAMS); + if (in) + SCTP_STRRESET_SET(asoc, SCTP_PARAM_RESET_ADD_IN_STREAMS); out: return retval;
@@ -564,13 +570,14 @@ struct sctp_chunk *sctp_process_strreset_outreq( if (asoc->strreset_chunk) { if (!sctp_chunk_lookup_strreset_param( asoc, outreq->response_seq, - SCTP_PARAM_RESET_IN_REQUEST)) { + SCTP_PARAM_RESET_IN_REQUEST) || + !SCTP_STRRESET_TEST(asoc, SCTP_PARAM_RESET_IN_REQUEST)) { /* same process with outstanding isn't 0 */ result = SCTP_STRRESET_ERR_IN_PROGRESS; goto out; } - asoc->strreset_outstanding--; + SCTP_STRRESET_CLEAR(asoc, SCTP_PARAM_RESET_IN_REQUEST); asoc->strreset_outseq++; if (!asoc->strreset_outstanding) {
@@ -669,7 +676,7 @@ struct sctp_chunk *sctp_process_strreset_inreq( SCTP_SO(stream, i)->state = SCTP_STREAM_CLOSED; asoc->strreset_chunk = chunk; - asoc->strreset_outstanding = 1; + SCTP_STRRESET_SET(asoc, SCTP_PARAM_RESET_OUT_REQUEST); sctp_chunk_hold(asoc->strreset_chunk); result = SCTP_STRRESET_PERFORMED;
@@ -816,13 +823,14 @@ struct sctp_chunk *sctp_process_strreset_addstrm_out( if (asoc->strreset_chunk) { if (!sctp_chunk_lookup_strreset_param( - asoc, 0, SCTP_PARAM_RESET_ADD_IN_STREAMS)) { + asoc, 0, SCTP_PARAM_RESET_ADD_IN_STREAMS) || + !SCTP_STRRESET_TEST(asoc, SCTP_PARAM_RESET_ADD_IN_STREAMS)) { /* same process with outstanding isn't 0 */ result = SCTP_STRRESET_ERR_IN_PROGRESS; goto out; } - asoc->strreset_outstanding--; + SCTP_STRRESET_CLEAR(asoc, SCTP_PARAM_RESET_ADD_IN_STREAMS); asoc->strreset_outseq++; if (!asoc->strreset_outstanding) {
@@ -899,7 +907,7 @@ struct sctp_chunk *sctp_process_strreset_addstrm_in( goto out; asoc->strreset_chunk = chunk; - asoc->strreset_outstanding = 1; + SCTP_STRRESET_SET(asoc, SCTP_PARAM_RESET_ADD_OUT_STREAMS); sctp_chunk_hold(asoc->strreset_chunk); stream->outcnt = outcnt;
@@ -928,7 +936,7 @@ struct sctp_chunk *sctp_process_strreset_resp( __u32 result; req = sctp_chunk_lookup_strreset_param(asoc, resp->response_seq, 0); - if (!req) + if (!req || !SCTP_STRRESET_TEST(asoc, req->type)) return NULL; result = ntohl(resp->result);
@@ -1078,7 +1086,7 @@ struct sctp_chunk *sctp_process_strreset_resp( nums, 0, GFP_ATOMIC); } - asoc->strreset_outstanding--; + SCTP_STRRESET_CLEAR(asoc, req->type); asoc->strreset_outseq++; /* remove everything for this reconf request */
--
2.55.0