Re: [PATCH net v3] net/smc: fix NULL dereference and UAF in smc_tcp_syn_recv_sock()

[Jiayuan Chen]

On 3/10/26 8:13 PM, Eric Dumazet wrote:
> On Tue, Mar 10, 2026 at 1:01 PM Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
> > From: Jiayuan Chen <jiayuan.chen@xxxxxxxxxx>
> >
> > Syzkaller reported a panic in smc_tcp_syn_recv_sock() [1].
> >
> > smc_tcp_syn_recv_sock() is called in the TCP receive path
> > (softirq) via icsk_af_ops->syn_recv_sock on the clcsock (TCP
> > listening socket). It reads sk_user_data to get the smc_sock
> > pointer. However, when the SMC listen socket is being closed
> > concurrently, smc_close_active() sets clcsock->sk_user_data
> > to NULL under sk_callback_lock, and then the smc_sock itself
> > can be freed via sock_put() in smc_release().
> >
> > This leads to two issues:
> >
> > 1) NULL pointer dereference: sk_user_data is NULL when
> >     accessed.
> > 2) Use-after-free: sk_user_data is read as non-NULL, but the
> >     smc_sock is freed before its fields (e.g., queued_smc_hs,
> >     ori_af_ops) are accessed.
> >
> > The race window looks like this:
> >
> >    CPU A (softirq)              CPU B (process ctx)
> >
> >    tcp_v4_rcv()
> >      TCP_NEW_SYN_RECV:
> >      sk = req->rsk_listener
> >      sock_hold(sk)
> >      /* No lock on listener */
> >                                 smc_close_active():
> >                                   write_lock_bh(cb_lock)
> >                                   sk_user_data = NULL
> >                                   write_unlock_bh(cb_lock)
> >                                   ...
> >                                   smc_clcsock_release()
> >                                   sock_put(smc->sk) x2
> >                                     -> smc_sock freed!
> >      tcp_check_req()
> >        smc_tcp_syn_recv_sock():
> >          smc = user_data(sk)
> >            -> NULL or dangling
> >          smc->queued_smc_hs
> >            -> crash!
>
>
> > diff --git a/net/smc/smc.h b/net/smc/smc.h
> > index 9e6af72784ba..8b3eabcdb542 100644
> > --- a/net/smc/smc.h
> > +++ b/net/smc/smc.h
> > @@ -342,8 +342,7 @@ static inline void smc_init_saved_callbacks(struct smc_sock *smc)
> >
> >   static inline struct smc_sock *smc_clcsock_user_data(const struct sock *clcsk)
> >   {
> > -       return (struct smc_sock *)
> > -              ((uintptr_t)clcsk->sk_user_data & ~SK_USER_DATA_NOCOPY);
> > +       return (struct smc_sock *)rcu_dereference_sk_user_data(clcsk);
> >   }
> Are you sure all smc_clcsock_user_data() callers  hold rcu_read_lock() ?
> In order to avoid surprises, I would have added a new helper.
>
>   static inline struct smc_sock *smc_clcsock_user_data_rcu(const struct
> sock *clcsk)
> ...
>
> to allow gradual conversion ?
>
> Thanks !

Sorry I missed that.

pw-bot: cr