Re: [PATCH net 1/1] sctp: stream: fully roll back denied add-stream state

On Wed, Jun 3, 2026 at 6:19 AM Ren Wei <[email protected]> wrote:
>
> From: Wyatt Feng <[email protected]>
>
> When ADD_OUT_STREAMS is denied, SCTP only shrinks the queued chunks and
> then lowers outcnt. That leaves removed stream metadata behind, so a
> later re-add can reuse a stale ext and hit a null-pointer dereference in
> the scheduler get path.
>
> Fix the rollback by tearing down the removed stream state the same way
> other stream resizes do. Unschedule the current scheduler state, clear
> out_curr if it still points at a removed stream, drop the removed stream
> ext state with sctp_stream_outq_migrate(), and then reschedule the
> remaining streams.
>
> This keeps scheduler-private RR/FC/PRIO lists consistent while fully
> rolling back denied outgoing stream additions.
>
> Fixes: 637784ade221 ("sctp: introduce priority based stream scheduler")
> Cc: [email protected]
> Reported-by: Yuan Tan <[email protected]>
> Reported-by: Yifan Wu <[email protected]>
> Reported-by: Juefei Pu <[email protected]>
> Reported-by: Zhengchuan Liang <[email protected]>
> Reported-by: Xin Liu <[email protected]>
> Assisted-by: Codex:GPT-5.4
> Signed-off-by: Wyatt Feng <[email protected]>
> Signed-off-by: Ren Wei <[email protected]>
> ---
>  net/sctp/stream.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index c2247793c88b..5c07c6148228 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -1037,6 +1037,7 @@ struct sctp_chunk *sctp_process_strreset_resp(
>                 *evp = sctp_ulpevent_make_assoc_reset_event(asoc, flags,
>                         stsn, rtsn, GFP_ATOMIC);
>         } else if (req->type == SCTP_PARAM_RESET_ADD_OUT_STREAMS) {
> +               const struct sctp_sched_ops *sched;
>                 struct sctp_strreset_addstrm *addstrm;
Thanks for the patch.

Please move 'sched' below 'addstrm' to follow the reverse Christmas tree
declaration style.

>                 __u16 number;
>
> @@ -1048,7 +1049,16 @@ struct sctp_chunk *sctp_process_strreset_resp(
>                         for (i = number; i < stream->outcnt; i++)
>                                 SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
>                 } else {
> -                       sctp_stream_shrink_out(stream, number);
> +                       sched = sctp_sched_ops_from_stream(stream);
> +                       sched->unsched_all(stream);
> +                       if (stream->out_curr)
> +                               for (i = number; i < stream->outcnt; i++)
> +                                       if (stream->out_curr == SCTP_SO(stream, i)) {
> +                                               stream->out_curr = NULL;
> +                                               break;
> +                                       }
I don’t think we need to update stream->out_curr here since it should never
end up pointing to a closed stream.

out_curr is only set in sctp_sched_dequeue_done(), and the stream_state ==
SCTP_STREAM_CLOSED check in sctp_outq_flush_data() already prevents it from
being assigned to a closed stream.

If the concern is the assignment in the sctp_chunk_abandoned() path, you
could just move the stream_state == SCTP_STREAM_CLOSED check above that to
make the intent clearer in sctp_outq_flush_data().

Thanks.

> +                       sctp_stream_outq_migrate(stream, NULL, number);
> +                       sched->sched_all(stream);
>                         stream->outcnt = number;
>                 }
>
> --
> 2.43.7
>