netdev - Re: [bpf PATCH v2 1/3] bpf: tls, implement unhash to avoid transition out of ESTABLISHED

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: <4f0cc6d0-1934-92d7-d2fd-d02ebe5ab822@gmail.com>
Date:   Thu, 25 Apr 2019 12:35:58 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     ast@...nel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [bpf PATCH v2 1/3] bpf: tls, implement unhash to avoid transition
 out of ESTABLISHED

On 4/25/19 12:32 PM, John Fastabend wrote:
> On 4/25/19 12:29 PM, Jakub Kicinski wrote:
>> On Thu, 25 Apr 2019 09:03:08 -0700, John Fastabend wrote:
>>> +static void tls_sk_proto_unhash(struct sock *sk)
>>> +{
>>> +	struct tls_context *ctx = tls_get_ctx(sk);
>>> +	void (*sk_proto_unhash)(struct sock *sk);
>>> +	bool free_ctx;
>>> +
>>> +	if (!ctx)
>>> +		return sk->sk_prot->unhash(sk);
>>> +	sk_proto_unhash = ctx->sk_proto_unhash;
>>> +	free_ctx = tls_sk_proto_destroy(sk, ctx, false);
>>> +	tls_put_ctx(sk);
>>
>> Oh, I think you can't put_ctx() unconditionally,
>> when free_ctx is false, tls_device_sk_destruct() 
>> needs it the ctx pointer.
>>
>> I think this explains the offload crashing.
>>
> 
> ugh yeah. So we need to _not_ free it from tls_sk_proto_destroy
> do the put_ctx and then finally free it. Otherwise we can't
> restore the sk_proto fields. v3 on its way. Thanks.
> 

I'm going to throw that patch I sent earlier in this thread
on the series as well. Its the minimal set to get things working
again for me. Will follow up some selftests so we don't get
here again.

>>> +	if (sk_proto_unhash)
>>> +		sk_proto_unhash(sk);
>>> +	if (free_ctx)
>>> +		tls_ctx_free(ctx);
>>> +}
>>>  
>>> -skip_tx_cleanup:
>>> +static void tls_sk_proto_close(struct sock *sk, long timeout)
>>> +{
>>> +	void (*sk_proto_close)(struct sock *sk, long timeout);
>>> +	struct tls_context *ctx = tls_get_ctx(sk);
>>> +	bool free_ctx;
>>> +
>>> +	if (!ctx)
>>> +		return sk->sk_prot->destroy(sk);
>>> +
>>> +	lock_sock(sk);
>>> +	sk_proto_close = ctx->sk_proto_close;
>>> +	free_ctx = tls_sk_proto_destroy(sk, ctx, true);
>>> +	tls_put_ctx(sk);
>