Re: [RFC PATCH v1 06/10] bpf: add plumbing for file-backed dynptr

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2025-10-03 at 17:04 +0100, Mykyta Yatsenko wrote:

[...]

> @@ -13969,7 +13982,12 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	 * PTR_TO_BTF_ID in bpf_kfunc_arg_meta, do the release now.
>  	 */
>  	if (meta.release_regno) {
> -		err = release_reference(env, regs[meta.release_regno].ref_obj_id);
> +		struct bpf_reg_state *reg = &regs[meta.release_regno];
> +
> +		if (meta.initialized_dynptr.ref_obj_id)
> +			err = unmark_stack_slots_dynptr(env, reg);
> +		else
> +			err = release_reference(env, reg->ref_obj_id);
>  		if (err) {
>  			verbose(env, "kfunc %s#%d reference has not been acquired before\n",
>  				func_name, meta.func_id);


Nit: unmark_stack_slots_dynptr() assumes that release_reference()
     inside it can't fail, which makes error report above unrelated
     for dynptrs. unmark_stack_slots_dynptr() can fail for other
     reasons, though.  Also, I think that condition should react on
     dynptr, not it's reg->ref_obj_id.
     So, I'd rewrite this hunk as follows:

        if (meta.release_regno) {
                struct bpf_reg_state *reg = &regs[meta.release_regno];

                if (meta.initialized_dynptr.type) {
                        err = unmark_stack_slots_dynptr(env, reg);
                        if (err)
                                return err;
                } else {
                        err = release_reference(env, ref_obj_id: reg->ref_obj_id);
                        if (err) {
                                verbose(private_data: env, fmt: "kfunc %s#%d reference has not been acquired before\n",
                                        func_name, meta.func_id);
                                return err;
                        }
                }
        }