From mboxrd@z Thu Jan  1 00:00:00 1970
From: John Fastabend <john.fastabend@gmail.com>
Subject: Re: [bpf PATCH v2 2/3] bpf: sockmap, fix transition through
 disconnect without close
Date: Mon, 17 Sep 2018 12:52:01 -0700
Message-ID: <7a85593b-7589-758e-2e5a-21c2ab1c6a5a@gmail.com>
References: <20180917175801.3870.61111.stgit@john-Precision-Tower-5810>
 <20180917175922.3870.69188.stgit@john-Precision-Tower-5810>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org
To: edumazet@google.com, ast@kernel.org, daniel@iogearbox.net
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-it0-f67.google.com ([209.85.214.67]:39138 "EHLO
        mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1727176AbeIRBVF (ORCPT
        <rfc822;netdev@vger.kernel.org>); Mon, 17 Sep 2018 21:21:05 -0400
Received: by mail-it0-f67.google.com with SMTP id h1-v6so12599117itj.4
        for <netdev@vger.kernel.org>; Mon, 17 Sep 2018 12:52:17 -0700 (PDT)
In-Reply-To: <20180917175922.3870.69188.stgit@john-Precision-Tower-5810>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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@google.com>
> Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
> Signed-off-by: John Fastabend <john.fastabend@gmail.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