netdev - Re: [PATCH net-next 6/6] ipv4/tcp: do not use per netns ctl sockets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6cccaaa7854c98248d663f60404ab6163250107f.camel@redhat.com>
Date:   Wed, 26 Jan 2022 17:14:42 +0100
From:   Paolo Abeni <pabeni@...hat.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     netdev <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>, mptcp@...ts.linux.dev
Subject: Re: [PATCH net-next 6/6] ipv4/tcp: do not use per netns ctl sockets

Hello,
On Mon, 2022-01-24 at 12:24 -0800, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@...gle.com>
> 
> TCP ipv4 uses per-cpu/per-netns ctl sockets in order to send
> RST and some ACK packets (on behalf of TIMEWAIT sockets).
> 
> This adds memory and cpu costs, which do not seem needed.
> Now typical servers have 256 or more cores, this adds considerable
> tax to netns users.
> 
> tcp sockets are used from BH context, are not receiving packets,
> and do not store any persistent state but the 'struct net' pointer
> in order to be able to use IPv4 output functions.
> 
> Note that I attempted a related change in the past, that had
> to be hot-fixed in commit bdbbb8527b6f ("ipv4: tcp: get rid of ugly unicast_sock")
> 
> This patch could very well surface old bugs, on layers not
> taking care of sk->sk_kern_sock properly.
> 
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>

We are observing UaF in our self-tests on top of this patch:

https://github.com/multipath-tcp/mptcp_net-next/issues/256

While I can't exclude the MPTCP code is misusing sk_net_refcnt and/or
sk_kern_sock, we can reproduce the issue even with plain TCP sockets[1]

The kasan report points to:

	struct inet_hashinfo *hashinfo = tw->tw_dr->hashinfo;

in inet_twsk_kill(). Apparently tw->tw_dr still refers to:

	&sock_net(sk)->ipv4.tcp_death_row

and the owning netns has been already dismantelled, as expected.
I could not find any code setting tw->tw_dr to a safe value after netns
destruction?!? am I missing something relevant?

Thanks!

Paolo

[1] patching the selftest script with the attached patch and running it
in a loop:

while ./mptcp_connect.sh -t -t; do : ; done

View attachment "selftests_tcp.patch" of type "text/x-patch" (1594 bytes)