Re: [syzbot] [sctp?] WARNING: refcount bug in sctp_transport_put (6)

[Xin Long]

On Tue, Sep 1, 2026 at 10:35 AM David Laight
<david.laight.linux@xxxxxxxxx> wrote:
>
> On Tue, 1 Sep 2026 09:45:42 -0400
> Xin Long <lucien.xin@xxxxxxxxx> wrote:
>
> > On Mon, Aug 31, 2026 at 11:47 PM xietangxin <xietangxin@xxxxxxxxxxxxxx> wrote:
> > >
> > > Hi,
> > >
> > > I have analyzed this issue and successfully reproduced locally.
> > > The race occurs between the timer callback (`sctp_generate_heartbeat_event`) and
> > > the transport cleanup path (`sctp_transport_free`):
> > >
> > > Task 1(Timer Softirq)               Task 2(sctp_transport_free)
> > > ==========================              ===============================
> > > sctp_generate_heartbeat_event()
> > >   refcnt = 2
> > >
> > >   bh_lock_sock(sk)
> > >   sock_owned_by_user(sk)
> > >   mod_timer(&hb_timer) -> returns 0
> > >                                           sctp_transport_free()
> > >                                             transport->dead = 1
> > >                                             del_timer(&hb_timer) -> returns 1!
> > >                                               sctp_transport_put() (2 -> 1)
> > >                                             sctp_transport_put() (1 -> 0)
> > >                                               sctp_transport_destroy()
> > >
> > >   sctp_transport_hold()
> > >     -> refcnt is 0, increment fails
> >
> > This should not be 0, as the transport must hold a refcnt to start the
> > hb_timer.
>
> Isn't there one hold sctp_generate_heartbeat_event() and a second for
> whatever 'task 2' is doing.
Right,

> When hb_timer is started it is given another hold (does it actually need one??).
You mean mod_timer() in sctp_generate_heartbeat_event()?  Yes, as it will
release the last one in out_unlock, it must hold a new one.

> So when hb_timer is deleted it's hold is removed.
> But the del_timer() is happening before the the extra hold is obtained.
>
Ahh, I can see the race now.

I remember you mentioned holding it before mod_reduce() in a previous
patch, maybe it will work here, like:

                sctp_transport_hold(transport);
                if (mod_timer(&transport->hb_timer, jiffies + (HZ/20)))
                        sctp_transport_put(transport);

Does it make sense?

Thanks.

> The RHS (task 2) would need to hold bh_lock_sock().
>
> Try giving sctp_generate_heartbeat_event() two holds.
>
> David
>
>
> >
> > Also, the delay below is under bh_lock_sock(), so it should not be the
> > real cause of the issue.
> >
> > Could you share the PoC for this issue?
> >
> > Thanks.
> >
> > >   out_unlock:
> > >     sctp_transport_put() (0 -> -1)
> > >     -> refcount underflow warning!
> > >
> > >
> > >
> > > Adding a small delay  after `mod_timer()` increases the reproduction rate:
> > >
> > > --- a/net/sctp/sm_sideeffect.c
> > > +++ b/net/sctp/sm_sideeffect.c
> > > @@ -373,8 +373,10 @@ void sctp_generate_heartbeat_event(struct timer_list *t)
> > >                 pr_debug("%s: sock is busy\n", __func__);
> > >
> > >                 /* Try again later.  */
> > > -               if (!mod_timer(&transport->hb_timer, jiffies + (HZ/20)))
> > > +               if (!mod_timer(&transport->hb_timer, jiffies + (HZ/20))) {
> > > +                       mdelay(1);
> > >                         sctp_transport_hold(transport);
> > > +               }
> > >                 goto out_unlock;
> > >         }
> > >
> > > Any feedback or guidance would be greatly appreciated.
> > >
> > > --
> > > Best regards,
> > > Tangxin Xie
> > >
> >
>