On Fri, Feb 14, 2025 at 8:45 AM Amery Hung <ameryhung@xxxxxxxxx> wrote:
>
> + if (!is_nullable && !is_refcounted)
> continue;
>
> + if (is_nullable)
> + suffix = MAYBE_NULL_SUFFIX;
> + else if (is_refcounted)
> + suffix = REFCOUNTED_SUFFIX;
I would remove the first 'if' and add:
else
continue;
> /* Should be a pointer to struct */
> pointed_type = btf_type_resolve_ptr(btf,
> args[arg_no].type,
> @@ -236,7 +246,7 @@ static int prepare_arg_info(struct btf *btf,
> if (!pointed_type ||
> !btf_type_is_struct(pointed_type)) {
> pr_warn("stub function %s has %s tagging to an unsupported type\n",
> - stub_fname, MAYBE_NULL_SUFFIX);
> + stub_fname, suffix);
> goto err_out;
> }
>
> @@ -254,11 +264,15 @@ static int prepare_arg_info(struct btf *btf,
> }
>
> /* Fill the information of the new argument */
> - info->reg_type =
> - PTR_TRUSTED | PTR_TO_BTF_ID | PTR_MAYBE_NULL;
> info->btf_id = arg_btf_id;
> info->btf = btf;
> info->offset = offset;
> + if (is_nullable) {
> + info->reg_type = PTR_TRUSTED | PTR_TO_BTF_ID | PTR_MAYBE_NULL;
> + } else if (is_refcounted) {
> + info->reg_type = PTR_TRUSTED | PTR_TO_BTF_ID;
> + info->refcounted = true;
> + }
>
> info++;
> info_cnt++;
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 9de6acddd479..fd3470fbd144 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6677,6 +6677,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> info->reg_type = ctx_arg_info->reg_type;
> info->btf = ctx_arg_info->btf ? : btf_vmlinux;
> info->btf_id = ctx_arg_info->btf_id;
> + info->ref_obj_id = ctx_arg_info->refcounted ? ctx_arg_info->ref_obj_id : 0;
> return true;
> }
> }
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a41ba019780f..a0f51903e977 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1543,6 +1543,17 @@ static void release_reference_state(struct bpf_verifier_state *state, int idx)
> return;
> }
>
> +static bool find_reference_state(struct bpf_verifier_state *state, int ptr_id)
> +{
> + int i;
> +
> + for (i = 0; i < state->acquired_refs; i++)
> + if (state->refs[i].id == ptr_id)
> + return true;
> +
> + return false;
> +}
> +
> static int release_lock_state(struct bpf_verifier_state *state, int type, int id, void *ptr)
> {
> int i;
> @@ -5981,7 +5992,8 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
> /* check access to 'struct bpf_context' fields. Supports fixed offsets only */
> static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off, int size,
> enum bpf_access_type t, enum bpf_reg_type *reg_type,
> - struct btf **btf, u32 *btf_id, bool *is_retval, bool is_ldsx)
> + struct btf **btf, u32 *btf_id, bool *is_retval, bool is_ldsx,
> + u32 *ref_obj_id)
It's ok-ish for now, but 11 arguments in a function is pushing it.
We've accumulated enough tech debt here.
Consider a follow up.
