Re: [PATCH bpf-next v3 5/6] bpf: sockmap: allow update from BPF

[Yonghong Song <yhs@fb.com>]

On 8/21/20 3:29 AM, Lorenz Bauer wrote:
> Allow calling bpf_map_update_elem on sockmap and sockhash from a BPF
> context. The synchronization required for this is a bit fiddly: we
> need to prevent the socket from changing its state while we add it
> to the sockmap, since we rely on getting a callback via
> sk_prot->unhash. However, we can't just lock_sock like in
> sock_map_sk_acquire because that might sleep. So instead we disable
> softirq processing and use bh_lock_sock to prevent further
> modification.
> 
> Yet, this is still not enough. BPF can be called in contexts where
> the current CPU might have locked a socket. If the BPF can get
> a hold of such a socket, inserting it into a sockmap would lead to
> a deadlock. One straight forward example are sock_ops programs that
> have ctx->sk, but the same problem exists for kprobes, etc.
> We deal with this by allowing sockmap updates only from known safe
> contexts. Improper usage is rejected by the verifier.
> 
> I've audited the enabled contexts to make sure they can't run in
> a locked context. It's possible that CGROUP_SKB and others are
> safe as well, but the auditing here is much more difficult. In
> any case, we can extend the safe contexts when the need arises.
> 
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>

Acked-by: Yonghong Song <yhs@fb.com>