Re: [PATCH net v2] sctp: validate stream count in sctp_process_strreset_inreq()

[David Laight <david.laight.linux@gmail.com>]

On Thu,  9 Jul 2026 21:07:18 -0400
"Cen Zhang (Microsoft)" <blbllhy@gmail.com> wrote:

> When processing a RESET_IN_REQUEST from a peer,
> sctp_process_strreset_inreq() derives the stream count from the
> parameter length but does not check whether the resulting
> RESET_OUT_REQUEST would exceed SCTP_MAX_CHUNK_LEN.
> 
> The OUT request header (sctp_strreset_outreq, 16 bytes) is 8 bytes
> larger than the IN request header (sctp_strreset_inreq, 8 bytes).
> Generally, the IP payload is bounded to 65535 bytes, so the stream
> list cannot be large enough to trigger the overflow. However, on
> interfaces with MTU > 65535 (e.g., loopback with IPv6 jumbograms), a
> stream list that fits within the incoming IN parameter can cause a
> __u16 overflow in sctp_make_strreset_req() when computing the OUT
> request size, leading to an undersized skb allocation and a kernel
> BUG:
> 
>   net/core/skbuff.c:207         skb_panic
>   net/core/skbuff.c:2625        skb_put
>   net/sctp/sm_make_chunk.c:1535 sctp_addto_chunk
>   net/sctp/sm_make_chunk.c:3695 sctp_make_strreset_req
>   net/sctp/stream.c:655         sctp_process_strreset_inreq
> 
> The local setsockopt path validates the generated reset request size.
> However, for an incoming-only reset, it accounts for the smaller IN
> request even though the peer must generate an OUT request with the same
> stream list. Such a request cannot be completed successfully by the
> peer.
> 
> Reject peer IN requests whose corresponding OUT request would exceed
> SCTP_MAX_CHUNK_LEN. Also tighten the local check so it does not send an
> IN request that would require an oversized OUT request from the peer.
> 
> Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset Request Parameter")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Closes: https://lore.kernel.org/all/20260707203215.2752-1-blbllhy@gmail.com/
> Suggested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> ---
> v2: Add the OUT request length check to the send path, as suggested by Xin Long.
> 
>  net/sctp/stream.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 5c2fdedea088..34ffe6c945a4 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -308,7 +308,8 @@ int sctp_send_reset_streams(struct sctp_association *asoc,
>  					goto out;
>  
>  			param_len += str_nums * sizeof(__u16) +
> -				     sizeof(struct sctp_strreset_inreq);
> +				     (out ? sizeof(struct sctp_strreset_inreq)
> +					  : sizeof(struct sctp_strreset_outreq));

Does it really make any sense to have a connection with the 32k streams
that would be needed in order to send a maximal length request?
(Or more likely a user requesting the same streams be reset multiple times.)
So an initial check that str_nums < SOME_CONSTANT_JUST_BELOW_32K would do.

Looking at the code I'm sure the kmalloc() shouldn't be done in the
'str_nums == 0' case either.
In fact it is probably worth doing the kmalloc() earlier to avoid two
scans of the array.
I even wonder if it should be possible to allocate the chunk without filling
in the data and then put the values in afterwards (freeing the chunk if there
is an error).

Then there is the code that reverts the state to OPEN if sctp_send_reconf()
fails - nothing check that is the original state.

	David
 

>  		}
>  
>  		if (param_len > SCTP_MAX_CHUNK_LEN -
> @@ -639,6 +640,9 @@ struct sctp_chunk *sctp_process_strreset_inreq(
>  
>  	nums = (ntohs(param.p->length) - sizeof(*inreq)) / sizeof(__u16);
>  	str_p = inreq->list_of_streams;
> +	if (nums * sizeof(__u16) + sizeof(struct sctp_strreset_outreq) >
> +	    SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_reconf_chunk))
> +		goto out;
>  	for (i = 0; i < nums; i++) {
>  		if (ntohs(str_p[i]) >= stream->outcnt) {
>  			result = SCTP_STRRESET_ERR_WRONG_SSN;