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

On Tue, Sep 1, 2026 at 10:35 AM David Laight
[off-list ref] wrote:

On Tue, 1 Sep 2026 09:45:42 -0400
Xin Long [off-list ref] wrote:

quoted

On Mon, Aug 31, 2026 at 11:47 PM xietangxin [off-list ref] wrote:

quoted

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

quoted

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.

quoted

  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