Re: [PATCH net] sctp: fix NULL deref on untransmitted RECONF completion

Xin Long <[email protected]> 于2026年8月21日周五 23:28写道：
>
> On Thu, Aug 20, 2026 at 12:23 PM <[email protected]> wrote:
> >
> > From: Weiming Shi <[email protected]>
> >
> > sctp_process_strreset_outreq(), sctp_process_strreset_addstrm_out() and
> > sctp_process_strreset_resp() finish a pending stream reconfiguration
> > request by stopping the reconf timer on the transport the request was
> > sent on:
> >
> >         t = asoc->strreset_chunk->transport;
> >         if (timer_delete(&t->reconf_timer))
> >                 sctp_transport_put(t);
> >
> > On the transmit path chunk->transport is assigned by
> > __sctp_packet_append_chunk(), that is, when the chunk is actually
> > appended to an outbound packet. A RECONF chunk that was queued but never
> > transmitted keeps transport == NULL, and its reconf timer is never armed
> > either, since that too only happens once the chunk has been appended.
> >
> > sctp_outq_flush_ctrl() skips every non-ASCONF control chunk while
> > asoc->src_out_of_asoc_ok is set and leaves it on control_chunk_list, so
> > in that state a RECONF is never put on the wire. The sender side has
> > already published the chunk in asoc->strreset_chunk and armed
> > asoc->strreset_outstanding, so an incoming RECONF that drives
> > strreset_outstanding down to 0 dereferences the NULL transport.
> >
> > sctp_send_asconf_del_ip() reaches that state without any peer
> > interaction: when the address being removed is the association's last
> > one it stashes the address in asoc->asconf_addr_del_pending, sets
> > src_out_of_asoc_ok and skips chunk creation and transmission
> > ("stored = 1; goto skip_mkasconf"). As only sctp_process_asconf_ack()
> > clears the flag, it stays set until a later bindx() ADD picks the pending
> > delete up. An unprivileged process that removes the last address of an
> > ASCONF-enabled association and then asks for a stream reset panics the
> > kernel from softirq:
> >
> >   Oops: general protection fault, probably for non-canonical address
> >   0xdffffc000000003d: 0000 [#1] SMP KASAN NOPTI
> >   KASAN: null-ptr-deref in range [0x00000000000001e8-0x00000000000001ef]
> >   RIP: 0010:timer_delete+0x67/0x110
> >   Call Trace:
> >    <IRQ>
> >    sctp_process_strreset_addstrm_out (net/sctp/stream.c:832)
> >    sctp_sf_do_reconf (net/sctp/sm_statefuns.c:4212)
> >    sctp_do_sm (net/sctp/sm_sideeffect.c:1172)
> >    sctp_assoc_bh_rcv (net/sctp/associola.c:1044)
> >    sctp_rcv (net/sctp/input.c:243)
> >    ip_protocol_deliver_rcu (net/ipv4/ip_input.c:207)
> >    ip_local_deliver (net/ipv4/ip_input.c:262)
> >    ip_rcv (net/ipv4/ip_input.c:612)
> >    process_backlog (net/core/dev.c:6680)
> >    net_rx_action (net/core/dev.c:7959)
> >    handle_softirqs (kernel/softirq.c:622)
> >    </IRQ>
> >   Kernel panic - not syncing: Fatal exception in interrupt
> >
> > Skip the timer deletion when the RECONF chunk never reached a packet:
> > there is no armed reconf timer and no transport reference to drop.
> >
> > Fixes: 810544764536 ("sctp: implement receiver-side procedures for the Outgoing SSN Reset Request Parameter")
> > Cc: [email protected]
> > Reported-by: Xiang Mei <[email protected]>
> > Assisted-by: Claude:claude-opus-5
> > Signed-off-by: Weiming Shi <[email protected]>
> > ---
> > Note to reviewers: this only stops the crash, and I want to be upfront that it
> > does not remove the state that produces it.
> >
> > That state has exactly one producer. sctp_send_asconf_del_ip() latches
> > src_out_of_asoc_ok on the branch that stashes the association's last address
> > without building or sending an ASCONF, and sctp_process_asconf_ack() is the
> > only place that clears it -- so the flag survives until the application issues
> > the follow-up bindx() ADD that sctp_make_asconf_update_ip()'s del_pickup needs.
> > 8a07eb0a50ae ("sctp: Add ASCONF operation on the single-homed host") describes
> > this as a transient state ("the other chunks cannot be transmitted in this
> > state"), but nothing bounds how long it lasts: an application that never adds a
> > replacement address leaves the association unable to send any DATA or
> > non-ASCONF control chunk for the rest of its life, with the RECONF sitting on
> > control_chunk_list and strreset_outstanding armed.
> >
> > I kept this patch to the dereference because it is what the rest of net/sctp
> > already does -- every other chunk->transport dereference is NULL-checked -- and
> > because it backports cleanly. But would you prefer the latch itself to be
> > addressed, or the reconf request to be abandoned here rather than completed
> > against a chunk that never went out? Happy to follow up with either for
> > net-next. net/sctp/stream.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> > index 34ffe6c945a4..1cb30def2eb9 100644
> > --- a/net/sctp/stream.c
> > +++ b/net/sctp/stream.c
> > @@ -577,7 +577,7 @@ struct sctp_chunk *sctp_process_strreset_outreq(
> >                         struct sctp_transport *t;
> >
> >                         t = asoc->strreset_chunk->transport;
> > -                       if (timer_delete(&t->reconf_timer))
> > +                       if (t && timer_delete(&t->reconf_timer))
> >                                 sctp_transport_put(t);
> >
> >                         sctp_chunk_put(asoc->strreset_chunk);
> > @@ -829,7 +829,7 @@ struct sctp_chunk *sctp_process_strreset_addstrm_out(
> >                         struct sctp_transport *t;
> >
> >                         t = asoc->strreset_chunk->transport;
> > -                       if (timer_delete(&t->reconf_timer))
> > +                       if (t && timer_delete(&t->reconf_timer))
> >                                 sctp_transport_put(t);
> >
> >                         sctp_chunk_put(asoc->strreset_chunk);
> > @@ -1084,7 +1084,7 @@ struct sctp_chunk *sctp_process_strreset_resp(
> >         /* remove everything for this reconf request */
> >         if (!asoc->strreset_outstanding) {
> >                 t = asoc->strreset_chunk->transport;
> > -               if (timer_delete(&t->reconf_timer))
> > +               if (t && timer_delete(&t->reconf_timer))
> >                         sctp_transport_put(t);
> >
> >                 sctp_chunk_put(asoc->strreset_chunk);
> > --
> > 2.55.0
> >
> If the STRRESETSTRRESET chunk has never been sent, we should NOT allow an incoming
> chunk to “ACK” it.
>
> Could you try fixing this by returning NULL from
> sctp_chunk_lookup_strreset_param() if the STRRESETSTRRESET chunk has not been
> transmitted yet? like:
>
> @@ -488,7 +488,7 @@ static struct sctp_paramhdr
> *sctp_chunk_lookup_strreset_param(
>         struct sctp_reconf_chunk *hdr;
>         union sctp_params param;
>
> -       if (!chunk)
> +       if (!chunk || !chunk->transport)
>                 return NULL;
>
> Please also share the PoC with the maintainers, if there is one.
>
> Thanks.

Hi,

Thanks for your review. Updated patch and PoC are sent.