On 2022/04/10 2:47, Eric Dumazet wrote: > So please add to your tree the NFS fix: > > commit f00432063db1a0db484e85193eccc6845435b80e > Author: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > Date: Sun Apr 3 15:58:11 2022 -0400 > > SUNRPC: Ensure we flush any closed sockets before xs_xprt_free() OK. Since the socket is sk->sk_net_refcnt=0, adding > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index e31cf137c6140f76f838b4a0dcddf9f104ad653b..3dacd202bf2af43c55ffe820c08316150d2018ea > 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -2928,6 +2928,8 @@ void tcp_close(struct sock *sk, long timeout) > lock_sock(sk); > __tcp_close(sk, timeout); > release_sock(sk); > + if (!sk->sk_net_refcnt) > + inet_csk_clear_xmit_timers_sync(sk); > sock_put(sk); > } > EXPORT_SYMBOL(tcp_close); part indeed helped avoiding use-after-free increment on sock_net(sk). But it seems to me that __sk_destruct() is forever not called. ---------------------------------------- [ 93.024086][ C1] sock: sk_clone_lock(): sk=ffff888110328000 net=ffff88810efb8000 sk->sk_family=10 sk->sk_net_refcnt=0 refcount_read(&net->ns.count)=2 [ 93.030257][ C1] sock: sk_clone_lock(): newsk=ffff888110350000 net=ffff88810efb8000 newsk->sk_family=10 newsk->sk_net_refcnt=0 refcount_read(&net->ns.count)=2 (...snipped...) [ 93.170750][ T740] TCP: Calling inet_csk_clear_xmit_timers_sync() on sock=ffff888110350000 (...snipped...) [ 214.272450][ T8] TCP: Calling inet_csk_clear_xmit_timers_sync() on sock=ffff888110328000 (...snipped...) [ 214.358528][ C3] sock: __sk_destruct(): sk=ffff888110328000 family=10 net=ffff88810efb8000 sk->sk_net_refcnt=0 ---------------------------------------- If I do - inet_csk_clear_xmit_timers_sync(sk); + write_pnet(&sk->sk_net, &init_net); in this patch (i.e. just avoid use-after-free access), __sk_destruct() is called when timer fires. ---------------------------------------- [ 81.969884][ C0] sock: sk_clone_lock(): sk=ffff8880156f8000 net=ffff8881030d8000 sk->sk_family=10 sk->sk_net_refcnt=0 refcount_read(&net->ns.count)=2 [ 81.975329][ C0] sock: sk_clone_lock(): newsk=ffff8880156f8c40 net=ffff8881030d8000 newsk->sk_family=10 newsk->sk_net_refcnt=0 refcount_read(&net->ns.count)=2 (...snipped...) [ 82.078152][ T735] TCP: Resetting sk->sk_net on sock=ffff8880156f8c40 (...snipped...) [ 203.937701][ T735] TCP: Resetting sk->sk_net on sock=ffff8880156f8000 (...snipped...) [ 204.042570][ C1] sock: __sk_destruct(): sk=ffff8880156f8000 family=10 net=ffffffff84588cc0 sk->sk_net_refcnt=0 (...snipped...) [ 214.124851][ C1] sock: __sk_destruct(): sk=ffff8880156f8c40 family=10 net=ffffffff84588cc0 sk->sk_net_refcnt=0 ---------------------------------------- Therefore, I guess that this patch is missing something here.
