On 06/04/2018 06:39 AM, Daniel Borkmann wrote: > Hey guys, > > On 06/02/2018 11:39 PM, John Fastabend wrote: >> 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! > > Given the discussion and fixes weren't resolved resp. ready in time for 4.17, > and last bpf pr for it went out last week, we need to route this via -stable > once all is hashed out. >
OK. > This fix here therefore needs to be rebased against bpf-next tree, and as far > as I can see another fix for hash map is also needed to address the same > issue. > This fix works for both sockmap and sockhash because they use the same ulp register and init paths. But, will rebase for net-next and send out this morning. > After that, likely also fixes for the disconnect + listen case are needed. > Yep will have a fix today for this. > (I can use the one here later on for -stable backport, but given merge window > is open this needs a rebase and a resolution for hash map.) > hash map is also resolved with the same patch but please do queue this up for -stable. > Thanks, > Daniel >
