Re: [PATCH net v2] tcp: clear sock_ops cb flags before force-closing a child socket

[Sechang Lim]

On Wed, Jun 10, 2026 at 11:11:49PM -0700, Kuniyuki Iwashima wrote:
> On Wed, Jun 10, 2026 at 9:00 AM Sechang Lim <rhkrqnwk98@xxxxxxxxx> wrote:
> > A child socket inherits the listener's bpf_sock_ops_cb_flags via
> > sk_clone_lock(). If its setup fails in tcp_v4_syn_recv_sock() /
> > tcp_v6_syn_recv_sock(), the child is freed through put_and_exit, where
> > inet_csk_prepare_forced_close() drops the socket lock and tcp_done() runs
> > without it.
> >
> > If BPF_SOCK_OPS_STATE_CB_FLAG was inherited, tcp_done() -> tcp_set_state()
> > calls tcp_call_bpf(), which expects the lock and trips sock_owned_by_me():
> >
> >   WARNING: include/net/sock.h:1799 at tcp_set_state+0x433/0x550
> >   RIP: 0010:tcp_set_state+0x433/0x550 include/net/sock.h:1799
> >   Call Trace:
> >    <IRQ>
> >    tcp_done+0xba/0x250 net/ipv4/tcp.c:5095
> >    tcp_v4_syn_recv_sock+0x850/0xa50 net/ipv4/tcp_ipv4.c:1787
> >    tcp_check_req+0xf30/0x1360 net/ipv4/tcp_minisocks.c:926
> >    tcp_v4_rcv+0x1047/0x1b50 net/ipv4/tcp_ipv4.c:2164
> >    </IRQ>
> >
> > The child is freed before it is ever established, so it should run no
> > sock_ops callback. Clear its cb flags in inet_csk_prepare_forced_close(),
> > which covers all forced-close callers.
> >
> > Suggested-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
> > Fixes: d44874910a26 ("bpf: Add BPF_SOCK_OPS_STATE_CB")
> > Signed-off-by: Sechang Lim <rhkrqnwk98@xxxxxxxxx>
> > ---
> > v2:
> >  - move the clearing into inet_csk_prepare_forced_close() (Jiayuan Chen,
> >    Simon Horman)
> >
> > v1:
> >  - https://lore.kernel.org/all/20260605094954.1374489-1-rhkrqnwk98@xxxxxxxxx/
> >
> >  include/net/tcp.h               | 7 +++++++
> >  net/ipv4/inet_connection_sock.c | 1 +
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 98848db62894..97eac5fa341c 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -2942,6 +2942,11 @@ static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
> >         return tcp_call_bpf(sk, op, 3, args);
> >  }
> >
> > +static inline void tcp_clear_sock_ops_cb_flags(struct sock *sk)
> > +{
> > +       tcp_sk(sk)->bpf_sock_ops_cb_flags = 0;
> > +}
> > +
> >  #else
> >  static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
> >  {
> > @@ -2959,6 +2964,8 @@ static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
> >         return -EPERM;
> >  }
> >
> > +static inline void tcp_clear_sock_ops_cb_flags(struct sock *sk) {}
>
> nit: I think prefered style is
>
> static inline void tcp_clear_sock_ops_cb_flags(struct sock *sk)
> {
> }

Will fix in v3.

> > +
> >  #endif
> >
> >  static inline u32 tcp_timeout_init(struct sock *sk)
> > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> > index dbcd37dfdc15..209f73a61a1d 100644
> > --- a/net/ipv4/inet_connection_sock.c
> > +++ b/net/ipv4/inet_connection_sock.c
> > @@ -1290,6 +1290,7 @@ void inet_csk_prepare_forced_close(struct sock *sk)
> >         __releases(&sk->sk_lock.slock)
> >  {
> >         /* sk_clone_lock locked the socket and set refcnt to 2 */
> > +       tcp_clear_sock_ops_cb_flags(sk);
>
> mptcp's ->syn_recv_sock() will trigger the hook in the failure path.
>
> inet_csk_prepare_for_destroy_sock() is a better fit.  We don't
> care if the child socket is locked in this path.

Makes sense, I missed the mptcp failure path. Moving it there in v3.

Thanks,
Sechang