On Sun, Mar 20, 2022 at 12:29:04AM +0530, Kumar Kartikeya Dwivedi wrote:
> > >
> > > if (is_release_function(func_id)) {
> > > - err = release_reference(env, meta.ref_obj_id);
> > > + err = -EINVAL;
> > > + if (meta.ref_obj_id)
> > > + err = release_reference(env, meta.ref_obj_id);
> > > + /* Only bpf_kptr_xchg is a release function that accepts a
> > > + * possibly NULL reg, hence meta.ref_obj_id can only be unset
> > > + * for it.
> >
> > Could you rephrase the comment? I'm not following what it's trying to convey.
> >
>
> All existing release helpers never take a NULL register, so their
> meta.ref_obj_id will never be unset, but bpf_kptr_xchg can, so it needs some
> special handling. In check_func_arg, when it jumps to skip_type_check label,
> reg->ref_obj_id won't be set for NULL value.
I still don't follow.
What do you mean 'unset meta.ref_obj_id' ?
It's either set or not.
meta->ref_obj_id will stay zero when arg == NULL.
Above 'if (meta.ref_obj_id)' makes sense.
But the code below with extra func_id check looks like defensive programming again.
> > > + */
> > > + else if (func_id == BPF_FUNC_kptr_xchg)
> > > + err = 0;
