mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Xin Long <lucien.xin@gmail.com>
To: Kyle Zeng <kylebot@openai.com>
Cc: linux-sctp@vger.kernel.org, marcelo.leitner@gmail.com,
	 netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net v2] sctp: avoid requeueing pending control chunks
Date: Tue, 3 Nov 2026 14:43:30 -0500	[thread overview]
Message-ID: <CADvbK_d3k8GsgjdHUZ0a2LDs-Y0jd3mxoFvLSnnz3WqCTbBbHA@mail.gmail.com> (raw)
In-Reply-To: <20261103192606.92502-1-kylebot@openai.com>

On Tue, Nov 3, 2026 at 2:26 PM Kyle Zeng <kylebot@openai.com> wrote:
>
> The T4 and RE-CONFIG timeout handlers take a reference to their cached
> control chunk and submit it through SCTP_CMD_REPLY. A previous
> transmission can still be on the control outqueue when output is corked
> or suppressed, or when sctp_packet_transmit_chunk() could not append the
> chunk.
>
> sctp_outq_tail() then adds the same list node a second time. Re-adding a
> non-tail entry can corrupt the list without being caught by the
> insertion-time list checks. A subsequent flush or association teardown
> can access an already freed chunk. The ASCONF path is reproducible by an
> unprivileged SCTP socket owner in a user/network namespace, with KASAN
> reporting a use-after-free in __sctp_outq_teardown().
>
> Check sctp_chunk_pending() in both timeout handlers before taking the
> extra reference and issuing SCTP_CMD_REPLY. When the chunk is already
> linked, request a deferred outqueue flush instead. Simply omitting the
> reply would also omit its uncork at the end of the timeout event. T4
> would no longer retry the queued ASCONF, and a migrated, pending
> RE-CONFIG could lose its timer until unrelated output flushes the queue.
>
> The flush command acquires a local cork only when the caller has not
> already corked the queue. This preserves the existing flush ordering,
> transport/error accounting and timer handling without changing the
> common enqueue path.
>
> The issue was found by an LLM.
>
> Fixes: 2e3216cd54b1 ("sctp: Follow security requirement of responding with 1 packet")
> Fixes: 7b9438de0cd4 ("sctp: add stream reconf timer")
> Cc: stable@kernel.org
> Suggested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> ---
> v1: https://lore.kernel.org/netdev/20261030042321.74407-1-kylebot@openai.com/
> v2: Check pending chunks in the T4 and RE-CONFIG timeout handlers, as
> requested. Preserve their deferred output flush with a state-machine
> command so pending requests can progress without unrelated traffic.
> The common sctp_outq_tail() path is unchanged.
>
> I have a reproducer for this issue and can share by DM.
>
>  include/net/sctp/command.h |  1 +
>  net/sctp/sm_sideeffect.c   |  8 ++++++++
>  net/sctp/sm_statefuns.c    | 20 ++++++++++++++------
>  3 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> index 2058fabffbf6..abd6cc509183 100644
> --- a/include/net/sctp/command.h
> +++ b/include/net/sctp/command.h
> @@ -39,6 +39,7 @@ enum sctp_verb {
>         SCTP_CMD_CHUNK_ULP,     /* Send a chunk to the sockets layer.  */
>         SCTP_CMD_EVENT_ULP,     /* Send a notification to the sockets layer. */
>         SCTP_CMD_REPLY,         /* Send a chunk to our peer.  */
> +       SCTP_CMD_OUTQ_FLUSH,    /* Flush pending output when uncorked. */
>         SCTP_CMD_SEND_PKT,      /* Send a full packet to our peer.  */
>         SCTP_CMD_RETRAN,        /* Mark a transport for retransmission.  */
>         SCTP_CMD_ECN_CE,        /* Do delayed CE processing.   */
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 35f540fb15fc..6f19ac78749d 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1498,6 +1498,14 @@ static int sctp_cmd_interpreter(enum sctp_event_type event_type,
>                         sctp_outq_tail(&asoc->outqueue, cmd->obj.chunk, gfp);
>                         break;
>
> +               case SCTP_CMD_OUTQ_FLUSH:
> +                       /* Request a flush without taking over an outer cork. */
> +                       if (!asoc->outqueue.cork) {
> +                               sctp_outq_cork(&asoc->outqueue);
> +                               local_cork = 1;
> +                       }
> +                       break;
> +
>                 case SCTP_CMD_SEND_PKT:
>                         /* Send a full packet to our peer.  */
>                         packet = cmd->obj.packet;
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index b2f609f177fb..8b76b7e28504 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1122,9 +1122,13 @@ enum sctp_disposition sctp_sf_send_reconf(struct net *net,
>                 return SCTP_DISPOSITION_DELETE_TCB;
>         }
>
> -       sctp_chunk_hold(asoc->strreset_chunk);
> -       sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> -                       SCTP_CHUNK(asoc->strreset_chunk));
> +       if (!sctp_chunk_pending(asoc->strreset_chunk)) {
> +               sctp_chunk_hold(asoc->strreset_chunk);
> +               sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> +                               SCTP_CHUNK(asoc->strreset_chunk));
> +       } else {
> +               sctp_add_cmd_sf(commands, SCTP_CMD_OUTQ_FLUSH, SCTP_NULL());
> +       }
>         sctp_add_cmd_sf(commands, SCTP_CMD_STRIKE, SCTP_TRANSPORT(transport));
>
>         return SCTP_DISPOSITION_CONSUME;
> @@ -6197,9 +6201,13 @@ enum sctp_disposition sctp_sf_t4_timer_expire(
>          * chunk, it MUST be the same (including its serial number) as the last
>          * ASCONF sent.
>          */
> -       sctp_chunk_hold(asoc->addip_last_asconf);
> -       sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> -                       SCTP_CHUNK(asoc->addip_last_asconf));
> +       if (!sctp_chunk_pending(asoc->addip_last_asconf)) {
> +               sctp_chunk_hold(asoc->addip_last_asconf);
> +               sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> +                               SCTP_CHUNK(asoc->addip_last_asconf));
> +       } else {
> +               sctp_add_cmd_sf(commands, SCTP_CMD_OUTQ_FLUSH, SCTP_NULL());
> +       }
>
>         /* ADDIP 4.1 B5) Restart the T-4 RTO timer. Note that if a different
>          * destination is selected, then the RTO used will be that of the new
>
I don't really understand why OUTQ_FLUSH is needed if the chunk
was not enqueued by this timeout.

Can you share the reproducer to help understand what caused the
chunk to not be transmitted on time?

Thanks.

      reply	other threads:[~2026-11-03 19:43 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 19:26 Kyle Zeng
2026-11-03 19:43 ` Xin Long [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADvbK_d3k8GsgjdHUZ0a2LDs-Y0jd3mxoFvLSnnz3WqCTbBbHA@mail.gmail.com \
    --to=lucien.xin@gmail.com \
    --cc=kylebot@openai.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®