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

[Xin Long]

On Thu, Jul 30, 2026 at 7:02 AM Jun Yang <juny24602@xxxxxxxxx> wrote:
>
> From: Jun Yang <junvyyang@xxxxxxxxxxx>
>
> 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.
>
> The function does not check asoc->strreset_outstanding on entry; it only
> decrements it at the tail, and asoc->strreset_chunk - the only thing
> sctp_chunk_lookup_strreset_param() consults - stays live until that
> counter reaches zero.  When both outgoing and incoming streams are added
> in one setsockopt(SCTP_ADD_STREAMS), sctp_send_add_streams() sets
> strreset_outstanding 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, and the lookup accepts any request still present in the
> cached chunk.  A peer can therefore 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.
>
> Only accept the response sequence currently named by strreset_outseq.
> Each accepted response advances that sequence, so a duplicate becomes
> stale even while another request parameter remains outstanding.  Also
> reject a response whose count exceeds the current stream count as a
> defence-in-depth check on the subtraction.
>
> Fixes: 11ae76e67a17 ("sctp: implement receiver-side procedures for the Reconf Response Parameter")
> Cc: stable@xxxxxxxxxx
> Reported-by: TencentOS Corvus AI <corvus@xxxxxxxxxxx>
> Signed-off-by: Jun Yang <junvyyang@xxxxxxxxxxx>
> ---
>  net/sctp/stream.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 34ffe6c945a4..b751d0843b3d 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -927,6 +927,14 @@ struct sctp_chunk *sctp_process_strreset_resp(
>         struct sctp_paramhdr *req;
>         __u32 result;
>
> +       /* Process responses in request sequence.  strreset_outseq advances after
> +        * each accepted response, so this also rejects duplicate responses while
> +        * another parameter from the same RECONF chunk remains outstanding.
> +        */
> +       if (!asoc-> strreset_outstanding ||
> +           resp->response_seq != htonl(asoc->strreset_outseq))
> +               return NULL;
> +
Hi, Jun, thanks for the patch.

Checking !asoc->strreset_outstanding doesn't really prevent the issue.
When both ADD_OUT and ADD_IN are sent in the same RE-CONFIG chunk,
strreset_outstanding is 2. After processing the ADD_OUT response, it
becomes 1. If a duplicate ADD_OUT response arrives before the ADD_IN
response, this check still passes.

Checking resp->response_seq != htonl(asoc->strreset_outseq) may break the
normal case. ADD_OUT uses outseq, while ADD_IN uses outseq + 1. If the
ADD_IN response arrives first, the check rejects a valid response.

>         req = sctp_chunk_lookup_strreset_param(asoc, resp->response_seq, 0);
>         if (!req)
>                 return NULL;
> @@ -1047,6 +1055,8 @@ struct sctp_chunk *sctp_process_strreset_resp(
>
>                 addstrm = (struct sctp_strreset_addstrm *)req;
>                 nums = ntohs(addstrm->number_of_streams);
> +               if (nums > stream->outcnt)
> +                       return NULL;

This prevents the underflow, but it's a defensive check rather than
addressing the root cause.

I think a better approach would be to make strreset_outstanding a bitmask
that tracks each outstanding request type independently, for example:

#define SCTP_STRRESET_MASK(type)        (1 << (ntohs(type) - 0x000d))
#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))

Then replace the existing strreset_outstanding increment/decrement logic
with these helpers. Especially, add a check in sctp_process_strreset_resp()
like:

req = sctp_chunk_lookup_strreset_param(asoc, resp->response_seq, 0);
if (!req || !SCTP_STRRESET_TEST(asoc, req->type))
        return NULL;

This way, once the response for a given request type has been processed,
any duplicate response for that type will be rejected, while responses for
other outstanding request types can still be accepted regardless of arrival
order.