netdev - Re: [PATCH 0/5] Return fds from privileged sockhash/sockmap lookup

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: <20200312015822.bhu6ptkx5jpabkr6@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 11 Mar 2020 18:58:22 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Lorenz Bauer <lmb@...udflare.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        kernel-team@...udflare.com, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH 0/5] Return fds from privileged sockhash/sockmap lookup

On Tue, Mar 10, 2020 at 05:47:06PM +0000, Lorenz Bauer wrote:
> We want to use sockhash and sockmap to build the control plane for
> our upcoming BPF socket dispatch work. We realised that it's
> difficult to resize or otherwise rebuild these maps if needed,
> because there is no way to get at their contents. This patch set
> allows a privileged user to retrieve fds from these map types,
> which removes this obstacle.
> 
> The approach here is different than that of program arrays and
> nested maps, which return an ID that can be turned into an fd
> using the BPF_*_GET_FD_BY_ID syscall. Sockets have IDs in the
> form of cookies, however there seems to be no way to go from
> a socket cookie to struct socket or struct file. Hence we
> return an fd directly.

we do store the socket FD into a sockmap, but returning new FD to that socket
feels weird. The user space suppose to hold those sockets. If it was bpf prog
that stored a socket then what does user space want to do with that foreign
socket? It likely belongs to some other process. Stealing it from other process
doesn't feel right.
Sounds like the use case is to take sockets one by one from one map, allocate
another map and store them there? The whole process has plenty of races. I
think it's better to tackle the problem from resize perspective. imo making it
something like sk_local_storage (which is already resizable pseudo map of
sockets) is a better way forward.