netdev - Re: [bpf PATCH v2 2/3] bpf: sockmap, fix transition through disconnect without close

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] [day] [month] [year] [list]

Message-ID: <7a85593b-7589-758e-2e5a-21c2ab1c6a5a@gmail.com>
Date:   Mon, 17 Sep 2018 12:52:01 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     edumazet@...gle.com, ast@...nel.org, daniel@...earbox.net
Cc:     netdev@...r.kernel.org
Subject: Re: [bpf PATCH v2 2/3] bpf: sockmap, fix transition through
 disconnect without close

On 09/17/2018 10:59 AM, John Fastabend wrote:
> It is possible (via shutdown()) for TCP socks to go trough TCP_CLOSE
> state via tcp_disconnect() without actually calling tcp_close which
> would then call our bpf_tcp_close() callback. Because of this a user
> could disconnect a socket then put it in a LISTEN state which would
> break our assumptions about sockets always being ESTABLISHED state.
> 
> To resolve this rely on the unhash hook, which is called in the
> disconnect case, to remove the sock from the sockmap.
> 

Sorry for the noise will need a v3 actually.

> Reported-by: Eric Dumazet <edumazet@...gle.com>
> Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
> Signed-off-by: John Fastabend <john.fastabend@...il.com>
> ---
>  kernel/bpf/sockmap.c |   71 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 52 insertions(+), 19 deletions(-)

[...]


> +}
> +
> +static void bpf_tcp_unhash(struct sock *sk)
> +{
> +	void (*unhash_fun)(struct sock *sk);
> +	struct smap_psock *psock;
> +
> +	rcu_read_lock();
> +	psock = smap_psock_sk(sk);
> +	if (unlikely(!psock)) {
> +		rcu_read_unlock();
> +		release_sock(sk);
                 ^^^^^^^^^^^^^^^^^
> +		return sk->sk_prot->unhash(sk);

		if (sk->sk_prot->unhash) ...
		else return;

Thanks,
John