On Wed, Sep 09, 2020 at 04:42:57PM +0100, Lorenz Bauer wrote: > 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. Thanks for checking!