Re: [bpf PATCH v2] bpf: sockmap, fix crash when ipv6 sock is added

[John Fastabend <john.fastabend@gmail.com>]

On 06/01/2018 12:58 PM, Eric Dumazet wrote:
> 
> 
> On 06/01/2018 03:46 PM, John Fastabend wrote:
>> This fixes a crash where we assign tcp_prot to IPv6 sockets instead
>> of tcpv6_prot.
> 
> ...
> 
>> +	/* ULPs are currently supported only for TCP sockets in ESTABLISHED
>> +	 * state. Supporting sockets in LISTEN state will require us to
>> +	 * modify the accept implementation to clone rather then share the
>> +	 * ulp context.
>> +	 */
>> +	if (sock->sk_state != TCP_ESTABLISHED)
>> +		return -ENOTSUPP;
>> +
>>  	/* 1. If sock map has BPF programs those will be inherited by the
>>  	 * sock being added. If the sock is already attached to BPF programs
>>  	 * this results in an error.
>>
> 
> Next question will be then : What happens if syzbot uses tcp_disconnect() and then listen() ?

Yep we need to fix that as well :( Looks like we can plumb the
unhash callback and remove it from the sockmap when the socket
goes through tcp_disconnect().

This patch should go in as-is though and we can fix the disconnect
issue with a new patch.

Adding Dave Watson to the thread as well because I'm guessing
the disconnect() case is also applicable to TLS. At least I see
a hw handler for unhash but there does not appear to be a handler
in the SW case, at least from a quick glance.

Thanks again!

> 
> Thanks !
>