[PATCH net v2] sctp: fix stream->outcnt underflow on duplicate RECONF responses

[PATCH net v2] sctp: fix stream->outcnt underflow on duplicate RECONF responses

[Jun Yang]

From: Jun Yang <junvyyang@tencent.com>

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 <corvus@tencent.com>
Suggested-by: Xin Long <lucien.xin@gmail.com>
Assisted-by: tencentos-corvus-ai:kimi-k3
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
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/

 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

Re: [PATCH net v2] sctp: fix stream->outcnt underflow on duplicate RECONF responses

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260804113100.37840-1-juny24602@gmail.com?part=1
---
--- Patch [1]: [PATCH net v2] sctp: fix stream->outcnt underflow on duplicate RECONF responses ---
commit 9a827b81b27b219603b09c1f04c9dbc9a6dd2368
Author: Jun Yang <junvyyang@tencent.com>

sctp: fix stream->outcnt underflow on duplicate RECONF responses

This commit addresses a bug where duplicate RECONF responses could cause
stream->outcnt to underflow. It converts strreset_outstanding into a bitmask
to track request parameter types, ensuring duplicate responses are ignored.

> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 34ffe6c945a4b..cb7543929d657 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c

[ ... ]

> @@ -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;

[Severity: High]
Could the new duplicate-drop logic permanently drop legitimate responses when
the sequence number wraps to 0?

If a user calls setsockopt(SCTP_ADD_STREAMS) with both incoming and outgoing
streams when asoc->strreset_outseq is exactly 0xFFFFFFFF, it creates a RECONF
chunk with request_seq = 0xFFFFFFFF for ADD_OUT and request_seq = 0 for
ADD_IN.

When the peer responds to the ADD_IN request with resp_seq = 0,
sctp_chunk_lookup_strreset_param() handles it:

net/sctp/stream.c:sctp_chunk_lookup_strreset_param() {
    ...
    if ((!resp_seq || req->request_seq == resp_seq) &&
        (!type || type == req->param_hdr.type))
            return param.v;
    ...
}

Because resp_seq is 0, the !resp_seq check bypasses sequence matching and
returns the first parameter in the chunk (ADD_OUT).

Since the ADD_OUT bit was already cleared by the first response, the new
!SCTP_STRRESET_TEST() check here would see the bit is cleared and drop the
response as a duplicate.

This would leave the ADD_IN bit set forever, preventing the RECONF chunk from
being freed and causing the association to hang due to endless
retransmissions.

Re: [PATCH net v2] sctp: fix stream->outcnt underflow on duplicate RECONF responses

[Xin Long]

On Wed, Aug 5, 2026 at 8:35 AM Simon Horman <horms@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev/#/patchset/20260804113100.37840-1-juny24602@gmail.com?part=1
> ---
> --- Patch [1]: [PATCH net v2] sctp: fix stream->outcnt underflow on duplicate RECONF responses ---
> commit 9a827b81b27b219603b09c1f04c9dbc9a6dd2368
> Author: Jun Yang <junvyyang@tencent.com>
>
> sctp: fix stream->outcnt underflow on duplicate RECONF responses
>
> This commit addresses a bug where duplicate RECONF responses could cause
> stream->outcnt to underflow. It converts strreset_outstanding into a bitmask
> to track request parameter types, ensuring duplicate responses are ignored.
>
> > diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> > index 34ffe6c945a4b..cb7543929d657 100644
> > --- a/net/sctp/stream.c
> > +++ b/net/sctp/stream.c
>
> [ ... ]
>
> > @@ -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;
>
> [Severity: High]
> Could the new duplicate-drop logic permanently drop legitimate responses when
> the sequence number wraps to 0?
>
> If a user calls setsockopt(SCTP_ADD_STREAMS) with both incoming and outgoing
> streams when asoc->strreset_outseq is exactly 0xFFFFFFFF, it creates a RECONF
> chunk with request_seq = 0xFFFFFFFF for ADD_OUT and request_seq = 0 for
> ADD_IN.
>
> When the peer responds to the ADD_IN request with resp_seq = 0,
> sctp_chunk_lookup_strreset_param() handles it:
>
> net/sctp/stream.c:sctp_chunk_lookup_strreset_param() {
>     ...
>     if ((!resp_seq || req->request_seq == resp_seq) &&
>         (!type || type == req->param_hdr.type))
>             return param.v;
>     ...
> }
>
> Because resp_seq is 0, the !resp_seq check bypasses sequence matching and
> returns the first parameter in the chunk (ADD_OUT).
>
> Since the ADD_OUT bit was already cleared by the first response, the new
> !SCTP_STRRESET_TEST() check here would see the bit is cleared and drop the
> response as a duplicate.
>
> This would leave the ADD_IN bit set forever, preventing the RECONF chunk from
> being freed and causing the association to hang due to endless
> retransmissions.

resp_seq == 0 is a valid value and must not be used to indicate a wildcard
lookup. This is a pre-existing issue, but it would be better to fix it before
this patch.

Hi Jun Yang,

Could you please address this by adding a 'bool match_seq' parameter to
sctp_chunk_lookup_strreset_param()?

After that, please repost this as a two-patch series:

1/2. Fix the lookup logic by introducing the match_seq parameter.
2/2. Apply the current fix on top of that change.

Thanks.