On Wed, 9 Sep 2020 at 10:16, Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote: > > On Tue, 8 Sep 2020 at 20:52, Martin KaFai Lau <kafai@xxxxxx> wrote: [...] > > Not all existing PTR_TO_SOCK_COMMON takes a reference also. > > Does it mean all these existing cases are broken? > > For example, bpf_sk_release(__sk_buff->sk) is allowed now? > > I'll look into this. It's very possible I got the refcounting logic > wrong, again. bpf_sk_release(__sk_buff->sk) is fine, and there is a test from Martin in verifier/sock.c that exercises this. The case I was worried about can't happen since release_reference_state returns EINVAL if it can't find a reference for the given ref_obj_id. Since we never allocate a reference with id 0 this ends up as the same thing, just less explicit than checking for id == 0. -- Lorenz Bauer | Systems Engineer 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK www.cloudflare.com