Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 29 Aug 2019 11:48:32 -0700
From:   John Fastabend <john.fastabend@gmail.com>
To:     Jakub Kicinski <jakub.kicinski@netronome.com>,
        Hillf Danton <hdanton@sina.com>
Cc:     john.fastabend@gmail.com,
        syzbot <syzbot+7a6ee4d0078eac6bf782@syzkaller.appspotmail.com>,
        aviadye@mellanox.com, borisp@mellanox.com, daniel@iogearbox.net,
        davejwatson@fb.com, davem@davemloft.net,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        syzkaller-bugs@googlegroups.com
Message-ID: <5d681e0011c7b_6b462ad11252c5c084@john-XPS-13-9370.notmuch>
In-Reply-To: <20190829094343.0248c61c@cakuba.netronome.com>
References: <000000000000c3c461059127a1c4@google.com>
 <20190829035200.3340-1-hdanton@sina.com>
 <20190829094343.0248c61c@cakuba.netronome.com>
Subject: Re: general protection fault in tls_sk_proto_close (2)
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Jakub Kicinski wrote:
> On Thu, 29 Aug 2019 11:52:00 +0800, Hillf Danton wrote:
> > Alternatively work is done if sock is closed again. Anyway ctx is reset
> > under sock's callback lock in write mode.
> > 
> > --- a/net/tls/tls_main.c
> > +++ b/net/tls/tls_main.c
> > @@ -295,6 +295,8 @@ static void tls_sk_proto_close(struct so
> >  	long timeo = sock_sndtimeo(sk, 0);
> >  	bool free_ctx;
> >  
> > +	if (!ctx)
> > +		return;
> >  	if (ctx->tx_conf == TLS_SW)
> >  		tls_sw_cancel_work_tx(ctx);
> 
> That's no bueno, the real socket's close will never get called.

Seems when we refactored BPF side we dropped the check for ULP on one
path so I'll add that back now. It would be nice and seems we are
getting closer now that tls side is a bit more dynamic if the ordering
didn't matter.

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 1330a7442e5b..30d11558740e 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -666,6 +666,8 @@ static int sock_hash_update_common(struct bpf_map *map, void *key,
        WARN_ON_ONCE(!rcu_read_lock_held());
        if (unlikely(flags > BPF_EXIST))
                return -EINVAL;
+       if (unlikely(icsk->icsk_ulp_data))
+               return -EINVAL;

        link = sk_psock_init_link();
        if (!link)