Re: [RFC PATCH v9 01/11] bpf: Support getting referenced kptr from struct_ops argument

[Martin KaFai Lau <martin.lau@linux.dev>]

On 7/14/24 10:51 AM, Amery Hung wrote:
> @@ -21004,6 +21025,13 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
>   		mark_reg_known_zero(env, regs, BPF_REG_1);
>   	}
>   
> +	if (env->prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
> +		ctx_arg_info = (struct bpf_ctx_arg_aux *)env->prog->aux->ctx_arg_info;
> +		for (i = 0; i < env->prog->aux->ctx_arg_info_size; i++)
> +			if (ctx_arg_info[i].refcounted)
> +				ctx_arg_info[i].ref_obj_id = acquire_reference_state(env, 0);
> +	}
> +

I think this will miss a case when passing the struct_ops prog ctx (i.e. "__u64 
*ctx") to a global subprog. Something like this:

__noinline int subprog_release(__u64 *ctx __arg_ctx)
{
	struct task_struct *task = (struct task_struct *)ctx[1];
	int dummy = (int)ctx[0];

	bpf_task_release(task);

	return dummy + 1;
}

SEC("struct_ops/subprog_ref")
__failure
int test_subprog_ref(__u64 *ctx)
{
	struct task_struct *task = (struct task_struct *)ctx[1];

	bpf_task_release(task);

	return subprog_release(ctx);;
}

SEC(".struct_ops.link")
struct bpf_testmod_ops subprog_ref = {
	.test_refcounted = (void *)test_subprog_ref,
};

A quick thought is, I think tracking the ctx's ref id in the env->cur_state may 
not be the correct place.

[ Just want to bring up what I have noticed so far. I will stop at here for 
today and will continue. ]