[PATCH net] sctp: preserve stream scheduler across outq teardown

[PATCH net] sctp: preserve stream scheduler across outq teardown

[Jun Yang]

From: Jun Yang <junvyyang@tencent.com>

sctp_outq_teardown() reinitializes a live outqueue with sctp_outq_init(),
which clears q->sched before calling sctp_sched_set_sched(). This makes
sctp_sched_set_sched() skip sctp_sched_free_sched(), although the
association's streams still retain their scheduler-private state. If the
socket's default scheduler differs from the installed one, the new
scheduler interprets that stale state through the shared union in
struct sctp_stream_out_ext.

Preserve q->sched across the reset so sctp_sched_set_sched() can free the
old scheduler's per-stream state and clear the union before installing
a different scheduler.

Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
Cc: stable@kernel.org
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
A KASAN reproducer for this issue is available if requested.

 net/sctp/outqueue.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index e92fb9da4647..6cb4b4339d8b 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -190,9 +190,13 @@ static inline int sctp_cacc_skip(struct sctp_transport *primary,
  */
 void sctp_outq_init(struct sctp_association *asoc, struct sctp_outq *q)
 {
+	const struct sctp_sched_ops *sched = q->sched;
+
 	memset(q, 0, sizeof(struct sctp_outq));
 
 	q->asoc = asoc;
+	/* Retain the scheduler to allow freeing its per-stream state. */
+	q->sched = sched;
 	INIT_LIST_HEAD(&q->out_chunk_list);
 	INIT_LIST_HEAD(&q->control_chunk_list);
 	INIT_LIST_HEAD(&q->retransmit);
-- 
2.54.0 (Apple Git-157)

Re: [PATCH net] sctp: preserve stream scheduler across outq teardown

[Xin Long]

On Wed, Nov 4, 2026 at 5:55 AM Jun Yang <juny24602@gmail.com> wrote:
>
> From: Jun Yang <junvyyang@tencent.com>
>
> sctp_outq_teardown() reinitializes a live outqueue with sctp_outq_init(),
> which clears q->sched before calling sctp_sched_set_sched(). This makes
> sctp_sched_set_sched() skip sctp_sched_free_sched(), although the
> association's streams still retain their scheduler-private state. If the
> socket's default scheduler differs from the installed one, the new
> scheduler interprets that stale state through the shared union in
> struct sctp_stream_out_ext.
>
> Preserve q->sched across the reset so sctp_sched_set_sched() can free the
> old scheduler's per-stream state and clear the union before installing
> a different scheduler.
>
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Cc: stable@kernel.org
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Signed-off-by: Jun Yang <junvyyang@tencent.com>
> ---
> A KASAN reproducer for this issue is available if requested.
>
>  net/sctp/outqueue.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index e92fb9da4647..6cb4b4339d8b 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -190,9 +190,13 @@ static inline int sctp_cacc_skip(struct sctp_transport *primary,
>   */
>  void sctp_outq_init(struct sctp_association *asoc, struct sctp_outq *q)
>  {
> +       const struct sctp_sched_ops *sched = q->sched;
> +
>         memset(q, 0, sizeof(struct sctp_outq));
>
>         q->asoc = asoc;
> +       /* Retain the scheduler to allow freeing its per-stream state. */
> +       q->sched = sched;
>         INIT_LIST_HEAD(&q->out_chunk_list);
>         INIT_LIST_HEAD(&q->control_chunk_list);
>         INIT_LIST_HEAD(&q->retransmit);

This doesn't look nice.
Can you try initializing the necessary members one by one instead of
memset(q, 0, sizeof(struct sctp_outq));? like:

          q->out_qlen = 0;
          q->error = 0;
          q->outstanding_bytes = 0;
          q->fast_rtx = 0;
          q->cork = 0;

Thanks.

Re: [PATCH net] sctp: preserve stream scheduler across outq teardown

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch changes how outqueue teardown behaves in the common
  case, and the commit message does not say so.

--

sctp: preserve stream scheduler across outq teardown

This patch keeps q->sched in sctp_outq_init() across the memset, so that
sctp_sched_set_sched() can free the old scheduler's per-stream state when
sctp_outq_teardown() reinitializes a live outqueue. Without that,
installing a different default scheduler could read stale data through
the shared union in struct sctp_stream_out_ext.

> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index e92fb9da4647a..6cb4b4339d8b7 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -190,9 +190,13 @@ static inline int sctp_cacc_skip(struct sctp_transport *primary,
>   */
>  void sctp_outq_init(struct sctp_association *asoc, struct sctp_outq *q)
>  {
> +	const struct sctp_sched_ops *sched = q->sched;
> +
>  	memset(q, 0, sizeof(struct sctp_outq));
>
>  	q->asoc = asoc;
> +	/* Retain the scheduler to allow freeing its per-stream state. */
> +	q->sched = sched;

[Severity: Medium]
The commit message only talks about the case where the installed scheduler
differs from sctp_sk(sk)->default_ss. Does this also change behaviour in
the more common case, where the two are the same?

Before this patch, q->sched was NULL after the memset. That meant
sctp_sched_set_sched() never took its early return, and every outqueue
purge ran n->init() and n->init_sid() on each stream:

net/sctp/stream_sched.c:sctp_sched_set_sched() {
    ...
	n = sctp_sched_ops[sched];
	if (old == n)
		return ret;
    ...
	n->init(&asoc->stream);
	for (i = 0; i < asoc->stream.outcnt; i++) {
		...
		ret = n->init_sid(&asoc->stream, i, GFP_ATOMIC);
    ...
}

With q->sched kept, old == n whenever default_ss matches the installed
scheduler, so the per-stream reset is skipped. The init_sid callbacks that
no longer run on teardown are:

net/sctp/stream_sched_prio.c:sctp_sched_prio_init_sid() {
	...
	return sctp_sched_prio_set(stream, sid, 0, gfp);
}

net/sctp/stream_sched_fc.c:sctp_sched_fc_init_sid() {
	...
	soute->fc_length = 0;
	soute->fc_weight = 1;
	...
}

As a result, PRIO priorities and WFQ weights set through
SCTP_STREAM_SCHEDULER_VALUE now survive a peer restart and a stale-cookie
retry. Previously both paths reset them to 0 and 1:

sctp_sf_do_dupcook_a() / sctp_sf_do_5_2_6_stale()
  -> SCTP_CMD_PURGE_OUTQUEUE
    -> sctp_outq_teardown()
      -> sctp_outq_init()
        -> sctp_sched_set_sched()   /* returns early, old == n */

For FC and WFQ there is a second effect. __sctp_outq_teardown() calls
sctp_sched_dequeue_done() on every leftover data chunk it discards:

net/sctp/outqueue.c:__sctp_outq_teardown() {
    ...
	while ((chunk = sctp_outq_dequeue_data(q)) != NULL) {
		sctp_sched_dequeue_done(q, chunk);

		/* Mark as send failure. */
		sctp_chunk_fail(chunk, q->error);
    ...
}

sctp_sched_fc_dequeue_done() then adds ch->skb->len to soute->fc_length.
That used to be cleared by init_sid. Now it isn't, so a stream with a
large unsent backlog at restart begins the restarted association with
fc_length inflated by bytes that were never sent.

Keeping the user-configured values may well be the preferred behaviour.
Since this is going to stable, could the commit message describe it? If
it isn't intended, should teardown reset the per-stream scheduler state
explicitly, or at least the fc_length accumulated from discarded chunks?

>  	INIT_LIST_HEAD(&q->out_chunk_list);
>  	INIT_LIST_HEAD(&q->control_chunk_list);
>  	INIT_LIST_HEAD(&q->retransmit);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104105507.62743-1-juny24602%40gmail.com

Re: [PATCH net] sctp: preserve stream scheduler across outq teardown

[Xin Long]

On Sun, Nov 8, 2026 at 6:51 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The patch changes how outqueue teardown behaves in the common
>   case, and the commit message does not say so.
>
> --
>
> sctp: preserve stream scheduler across outq teardown
>
> This patch keeps q->sched in sctp_outq_init() across the memset, so that
> sctp_sched_set_sched() can free the old scheduler's per-stream state when
> sctp_outq_teardown() reinitializes a live outqueue. Without that,
> installing a different default scheduler could read stale data through
> the shared union in struct sctp_stream_out_ext.
>
> > diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> > index e92fb9da4647a..6cb4b4339d8b7 100644
> > --- a/net/sctp/outqueue.c
> > +++ b/net/sctp/outqueue.c
> > @@ -190,9 +190,13 @@ static inline int sctp_cacc_skip(struct sctp_transport *primary,
> >   */
> >  void sctp_outq_init(struct sctp_association *asoc, struct sctp_outq *q)
> >  {
> > +     const struct sctp_sched_ops *sched = q->sched;
> > +
> >       memset(q, 0, sizeof(struct sctp_outq));
> >
> >       q->asoc = asoc;
> > +     /* Retain the scheduler to allow freeing its per-stream state. */
> > +     q->sched = sched;
>
> [Severity: Medium]
> The commit message only talks about the case where the installed scheduler
> differs from sctp_sk(sk)->default_ss. Does this also change behaviour in
> the more common case, where the two are the same?
>
> Before this patch, q->sched was NULL after the memset. That meant
> sctp_sched_set_sched() never took its early return, and every outqueue
> purge ran n->init() and n->init_sid() on each stream:
>
> net/sctp/stream_sched.c:sctp_sched_set_sched() {
>     ...
>         n = sctp_sched_ops[sched];
>         if (old == n)
>                 return ret;
>     ...
>         n->init(&asoc->stream);
>         for (i = 0; i < asoc->stream.outcnt; i++) {
>                 ...
>                 ret = n->init_sid(&asoc->stream, i, GFP_ATOMIC);
>     ...
> }
>
> With q->sched kept, old == n whenever default_ss matches the installed
> scheduler, so the per-stream reset is skipped. The init_sid callbacks that
> no longer run on teardown are:
>
> net/sctp/stream_sched_prio.c:sctp_sched_prio_init_sid() {
>         ...
>         return sctp_sched_prio_set(stream, sid, 0, gfp);
> }
>
> net/sctp/stream_sched_fc.c:sctp_sched_fc_init_sid() {
>         ...
>         soute->fc_length = 0;
>         soute->fc_weight = 1;
>         ...
> }
>
> As a result, PRIO priorities and WFQ weights set through
> SCTP_STREAM_SCHEDULER_VALUE now survive a peer restart and a stale-cookie
> retry. Previously both paths reset them to 0 and 1:
>
> sctp_sf_do_dupcook_a() / sctp_sf_do_5_2_6_stale()
>   -> SCTP_CMD_PURGE_OUTQUEUE
>     -> sctp_outq_teardown()
>       -> sctp_outq_init()
>         -> sctp_sched_set_sched()   /* returns early, old == n */
>
> For FC and WFQ there is a second effect. __sctp_outq_teardown() calls
> sctp_sched_dequeue_done() on every leftover data chunk it discards:
>
> net/sctp/outqueue.c:__sctp_outq_teardown() {
>     ...
>         while ((chunk = sctp_outq_dequeue_data(q)) != NULL) {
>                 sctp_sched_dequeue_done(q, chunk);
>
>                 /* Mark as send failure. */
>                 sctp_chunk_fail(chunk, q->error);
>     ...
> }
>
> sctp_sched_fc_dequeue_done() then adds ch->skb->len to soute->fc_length.
> That used to be cleared by init_sid. Now it isn't, so a stream with a
> large unsent backlog at restart begins the restarted association with
> fc_length inflated by bytes that were never sent.
>
That's expected. purged chunks were queued on those streams,
so the fc_length stays consistent and converges as new data flows.

You may just need to mention it in the commit message when
addressing my last comment.

Thanks.

> Keeping the user-configured values may well be the preferred behaviour.
> Since this is going to stable, could the commit message describe it? If
> it isn't intended, should teardown reset the per-stream scheduler state
> explicitly, or at least the fc_length accumulated from discarded chunks?
>
> >       INIT_LIST_HEAD(&q->out_chunk_list);
> >       INIT_LIST_HEAD(&q->control_chunk_list);
> >       INIT_LIST_HEAD(&q->retransmit);
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104105507.62743-1-juny24602%40gmail.com