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

On Tue, Jul 7, 2026 at 4:32 PM Cen Zhang (Microsoft) <[email protected]> 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 response 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 response size, leading to
> an undersized skb allocation, raising 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 (sctp_send_reset_streams) already performs length
> validation, but the network packet path does not. Fix by adding similar
> length check before calling sctp_make_strreset_req().
>
> Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset
> Request Parameter")
> Reported-by: [email protected]
> Signed-off-by: Cen Zhang (Microsoft) <[email protected]>
> ---
>  net/sctp/stream.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 5c2fdedea..ea3805712 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -639,6 +639,10 @@ 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;
> --
> 2.53.0
>
I think we should also prevent sending such an 'inreq', since it will
always be rejected by the peer. We can add improve the check in
'sctp_send_reset_streams()' like:

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index ea3805712b76..51a14d1f2391 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));
                }

Nits: Please keep the '>' on the same line as the left-hand operand and
indent the continuation line using the usual kernel style. Also No braces
are needed for a single statement.

if (nums * sizeof(__u16) + sizeof(struct sctp_strreset_outreq) >
    SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_reconf_chunk))
        goto out;

Thanks.