March 9, 2026 at 14:06, "D. Wythe" <alibuda@linux.alibaba.com mailto:alibuda@linux.alibaba.com?to=%22D.%20Wythe%22%20%3Calibuda%40linux.alibaba.com%3E > wrote:


[...]

quoted

 Reproducer was verified with mdelay injection and smc_run,
 the issue no longer occurs with this patch applied.
 
 [1] https://syzkaller.appspot.com/bug?extid=827ae2bfb3a3529333e9
 
 Fixes: 8270d9c21041 ("net/smc: Limit backlog connections")
 Reported-by: syzbot+827ae2bfb3a3529333e9@syzkaller.appspotmail.com
 Closes: https://lore.kernel.org/all/67eaf9b8.050a0220.3c3d88.004a.GAE@google.com/T/ (local)
 Suggested-by: Eric Dumazet [off-list ref]
 Signed-off-by: Jiayuan Chen [off-list ref]
 ---
 v2:
 - Use rcu_read_lock() + refcount_inc_not_zero() instead of
 read_lock_bh(sk_callback_lock) + sock_hold(), since this
 is the TCP handshake hot path and read_lock_bh is too
 expensive under SYN flood.
 - Set SOCK_RCU_FREE on SMC listen socket to ensure
 RCU-deferred freeing.
 
 v1: https://lore.kernel.org/netdev/20260307032158.372165-1-jiayuan.chen@linux.dev/ (local)
 ---
 net/smc/af_smc.c | 9 +++++++++
 1 file changed, 9 insertions(+)
 
 diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
 index d0119afcc6a1..72ac1d8c62d4 100644
 --- a/net/smc/af_smc.c
 +++ b/net/smc/af_smc.c
 @@ -131,7 +131,13 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
 struct smc_sock *smc;
 struct sock *child;
 
 + rcu_read_lock();
 smc = smc_clcsock_user_data(sk);
 + if (!smc || !refcount_inc_not_zero(&smc->sk.sk_refcnt)) {
 + rcu_read_unlock();
 + return NULL;
 + }
 + rcu_read_unlock();
 
 if (READ_ONCE(sk->sk_ack_backlog) + atomic_read(&smc->queued_smc_hs) >
 sk->sk_max_ack_backlog)
 @@ -153,11 +159,13 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
 if (inet_csk(child)->icsk_af_ops == inet_csk(sk)->icsk_af_ops)
 inet_csk(child)->icsk_af_ops = smc->ori_af_ops;
 }
 + sock_put(&smc->sk);
 return child;
 
 drop:
 dst_release(dst);
 tcp_listendrop(sk);
 + sock_put(&smc->sk);
 return NULL;
 }
 
 @@ -2691,6 +2699,7 @@ int smc_listen(struct socket *sock, int backlog)
 write_unlock_bh(&smc->clcsock->sk->sk_callback_lock);
 goto out;
 }
 + sock_set_flag(sk, SOCK_RCU_FREE);

This RCU approach looks good to me. Since SOCK_RCU_FREE is now enabled,
other callers of smc_clcsock_user_data() should also follow this
RCU-based pattern. It will eventually allow us to completely remove the
annoying sk_callback_lock.

D. Wythe

Hi D. Wythe,

Thanks for the suggestion. I agree that converting all smc_clcsock_user_data()
callers to RCU is a reasonable direction, and it would allow us to eventually
remove the sk_callback_lock dependency for sk_user_data access.

However, I'd prefer to keep this patch focused on fixing the specific bug in
smc_tcp_syn_recv_sock(), since it needs to be backported to stable trees.
Mixing a bug fix with broader refactoring makes backporting harder and increases
the risk of regressions.

Also, converting the other callers is not entirely trivial. For example:

- smc_fback_state_change/data_ready/write_space/error_report():
  the sk_callback_lock there protects not only sk_user_data but also the
  consistency of the saved callback pointers (e.g.,smc->clcsk_state_change).
  Switching to RCU requires careful ordering analysis against the write side
  in smc_fback_restore_callbacks(). Additionally, fallback sockets would also
  need SOCK_RCU_FREE.

- smc_clcsock_data_ready():
  the sock_hold() would need to become refcount_inc_not_zero() to handle the
  case where the refcount has already reached zero.

I'd like to address these conversions in a follow-up patch series.

What do you think?

quoted

sk->sk_max_ack_backlog = backlog;
 sk->sk_ack_backlog = 0;
 sk->sk_state = SMC_LISTEN;
 -- 
 2.43.0