[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <78cdbf25-4511-a567-bb09-0c07edae8b50@I-love.SAKURA.ne.jp>
Date: Sun, 10 Apr 2022 20:36:02 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Eric Dumazet <edumazet@...gle.com>
Cc: bpf <bpf@...r.kernel.org>,
syzbot <syzbot+694120e1002c117747ed@...kaller.appspotmail.com>,
Andrii Nakryiko <andrii@...nel.org>,
Andrii Nakryiko <andriin@...com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
David Miller <davem@...emloft.net>,
David Ahern <dsahern@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Martin KaFai Lau <kafai@...com>,
KP Singh <kpsingh@...nel.org>,
Jakub Kicinski <kuba@...nel.org>,
Alexey Kuznetsov <kuznet@....inr.ac.ru>,
netdev <netdev@...r.kernel.org>,
Song Liu <songliubraving@...com>,
syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
tpa@...hospital.com, Yonghong Song <yhs@...com>,
Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Trond Myklebust <trondmy@...merspace.com>
Subject: Re: [syzbot] KASAN: use-after-free Read in tcp_retransmit_timer (5)
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@...merspace.com>
> 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.
Powered by blists - more mailing lists