netdev - RE: [RFC bpf-next 0/5] Extend SOCKMAP to store listening sockets

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: <5db1d7a810bdb_5c282ada047205c08f@john-XPS-13-9370.notmuch>
Date:   Thu, 24 Oct 2019 09:56:08 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Jakub Sitnicki <jakub@...udflare.com>, bpf@...r.kernel.org
Cc:     John Fastabend <john.fastabend@...il.com>,
        Martin KaFai Lau <kafai@...com>, netdev@...r.kernel.org,
        kernel-team@...udflare.com
Subject: RE: [RFC bpf-next 0/5] Extend SOCKMAP to store listening sockets

Jakub Sitnicki wrote:
> This patch set is a follow up on a suggestion from LPC '19 discussions to
> make SOCKMAP (or a new map type derived from it) a generic type for storing
> established as well as listening sockets.
> 
> We found ourselves in need of a map type that keeps references to listening
> sockets when working on making the socket lookup programmable, aka BPF
> inet_lookup [1].  Initially we repurposed REUSEPORT_SOCKARRAY but found it
> problematic to extend due to being tightly coupled with reuseport
> logic (see slides [2]). So we've turned our attention to SOCKMAP instead.
> 
> As it turns out the changes needed to make SOCKMAP suitable for storing
> listening sockets are self-contained and have use outside of programming
> the socket lookup. Hence this patch set.
> 
> With these patches SOCKMAP can be used in SK_REUSEPORT BPF programs as a
> drop-in replacement for REUSEPORT_SOCKARRAY for TCP. This can hopefully
> lead to code consolidation between the two map types in the future.
> 
> Having said that, the main intention here is to lay groundwork for using
> SOCKMAP in the next iteration of programmable socket lookup patches.
> 
> I'm looking for feedback if there's anything fundamentally wrong with
> extending SOCKMAP map type like this that I might have missed.

I think this looks good. The main reason I blocked it off before is mostly
because I had no use-case for it and the complication with what to do with
child sockets. Clearing the psock state seems OK to me if user wants to
add it back to a map they can simply grab it again from a sockops event.

By the way I would eventually like to see the lookup hook return the
correct type (PTR_TO_SOCKET_OR_NULL) so that the verifier "knows" the type
and the socket can be used the same as if it was pulled from a sk_lookup
helper.

> 
> Thanks,
> Jakub
> 
> [1] https://lore.kernel.org/bpf/20190828072250.29828-1-jakub@cloudflare.com/
> [2] https://linuxplumbersconf.org/event/4/contributions/487/attachments/238/417/Programmable_socket_lookup_LPC_19.pdf
>