* [PATCH net v2] sctp: avoid requeueing pending control chunks
@ 2026-11-03 19:26 Kyle Zeng
2026-11-03 19:43 ` Xin Long
0 siblings, 1 reply; 2+ messages in thread
From: Kyle Zeng @ 2026-11-03 19:26 UTC (permalink / raw)
To: linux-sctp; +Cc: lucien.xin, marcelo.leitner, netdev, linux-kernel
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
base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
--
2.55.0.openai.911.g06873a30d2f3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net v2] sctp: avoid requeueing pending control chunks
2026-11-03 19:26 [PATCH net v2] sctp: avoid requeueing pending control chunks Kyle Zeng
@ 2026-11-03 19:43 ` Xin Long
0 siblings, 0 replies; 2+ messages in thread
From: Xin Long @ 2026-11-03 19:43 UTC (permalink / raw)
To: Kyle Zeng; +Cc: linux-sctp, marcelo.leitner, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-03 19:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 19:26 [PATCH net v2] sctp: avoid requeueing pending control chunks Kyle Zeng
2026-11-03 19:43 ` Xin Long
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox