On Wed, Mar 2, 2022 at 3:00 PM Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote: > > > > fwiw I like patches 1-3. > > I think extra check here for release func is justified on its own. > > Converting it into: > > fixed_off_ok = false; > > if (type == PTR_TO_BTF_ID && (!is_release_func || !reg->ref_obj_id)) > > fixed_off_ok = true; > > obfuscates the check to me. > > I was talking of putting this inside check_func_arg_reg_off. I think we should > do the same check for BPF helpers as well (rn only one supports releasing > PTR_TO_BTF_ID, soon we may have others). Just passing a bool to > check_func_arg_reg_off to indicate we are checking for release func (helper or > kfunc have same rules here) would allow putting this check inside it. Hmm. check_func_arg() is called before we call is_release_function(func_id). Are you proposing to call it before and pass another boolean into check_func_arg() or store the flag in meta? Sounds ugly. imo reg->off is a simple enough check to keep it open coded where necessary.
