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
| ||
|
Date: Mon, 4 Jun 2018 06:57:28 -0700 From: John Fastabend <john.fastabend@...il.com> To: Daniel Borkmann <daniel@...earbox.net>, Eric Dumazet <eric.dumazet@...il.com>, edumazet@...gle.com, ast@...nel.org, Dave Watson <davejwatson@...com> Cc: netdev@...r.kernel.org Subject: Re: [bpf PATCH v2] bpf: sockmap, fix crash when ipv6 sock is added 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 >
Powered by blists - more mailing lists