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

[David Laight]

On Tue, 1 Sep 2026 11:06:31 -0400
Xin Long <lucien.xin@xxxxxxxxx> wrote:

> 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.

But can that ever be the last 'hold' that actually calls sctp_transport_destroy().
It the timer is always deleted (as task 2 above) it doesn't need one itself.
Might need to be del_timer_sync() so that it waits for the completion function
to terminate.

> 
> > 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?

That should close the timing window, but is probably inefficient.
Rather depends on how often the 'put' ends up being done.

David

> 
> 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
> > > >  
> > >  
> >  
>