The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net v2] sctp: validate stream count in sctp_process_strreset_inreq()
@ 2026-07-10  1:07 Cen Zhang (Microsoft)
  2026-07-10  8:25 ` David Laight
  0 siblings, 1 reply; 3+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-07-10  1:07 UTC (permalink / raw)
  To: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni
  Cc: horms, linux-sctp, netdev, linux-kernel, AutonomousCodeSecurity,
	tgopinath, kys, blbllhy

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));
 		}
 
 		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;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] sctp: validate stream count in sctp_process_strreset_inreq()
  2026-07-10  1:07 [PATCH net v2] sctp: validate stream count in sctp_process_strreset_inreq() Cen Zhang (Microsoft)
@ 2026-07-10  8:25 ` David Laight
  2026-07-10 15:34   ` Xin Long
  0 siblings, 1 reply; 3+ messages in thread
From: David Laight @ 2026-07-10  8:25 UTC (permalink / raw)
  To: Cen Zhang (Microsoft)
  Cc: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni, horms,
	linux-sctp, netdev, linux-kernel, AutonomousCodeSecurity,
	tgopinath, kys

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;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] sctp: validate stream count in sctp_process_strreset_inreq()
  2026-07-10  8:25 ` David Laight
@ 2026-07-10 15:34   ` Xin Long
  0 siblings, 0 replies; 3+ messages in thread
From: Xin Long @ 2026-07-10 15:34 UTC (permalink / raw)
  To: David Laight
  Cc: Cen Zhang (Microsoft), marcelo.leitner, davem, edumazet, kuba,
	pabeni, horms, linux-sctp, netdev, linux-kernel,
	AutonomousCodeSecurity, tgopinath, kys

On Fri, Jul 10, 2026 at 4:26 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> 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.
>
Yes, that would be a simpler fix.

However, 32K is not a limit defined by the RFC. While 32K streams may seem
excessive in practice, we cannot say that such a configuration is invalid.
If an application legitimately needs more than that, it would be difficult
to argue that it is not using SCTP correctly.

> 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.
>
That would be another issue that we can address separately.
It would be great if you could follow up on this one. :-)

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

Acked-by: Xin Long <lucien.xin@gmail.com>

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-10 15:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10  1:07 [PATCH net v2] sctp: validate stream count in sctp_process_strreset_inreq() Cen Zhang (Microsoft)
2026-07-10  8:25 ` David Laight
2026-07-10 15:34   ` Xin Long

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox