Re: [PATCH v2 1/1] bpf: reject overlarge global subprog argument sizes

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c8d980fdd709..9248ba0f8089 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6924,6 +6924,11 @@ static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg
>  	struct bpf_reg_state saved_reg;
>  	int err;
>
> +	if (mem_size > S32_MAX) {
> +		verbose(env, "R%d memory size %u is too large\n", regno, mem_size);
> +		return -EACCES;
> +	}
> +

Does this hunk build?  The function signature is:

  static int check_mem_reg(struct bpf_verifier_env *env,
                           struct bpf_reg_state *reg,
                           argno_t argno, u32 mem_size)

There is no parameter or local named regno here, only argno (of typedef
argno_t).  Other helpers in this file that need a register number
compute it locally, e.g.:

  int regno = reg_from_argno(argno);

Without that, the verbose() call references an undeclared identifier
and the file fails to compile.

The selftest below expects the literal text "R1 memory size 4294967292
is too large", so the format string presumably wants the integer
register number printed.  Would something like:

  int regno = reg_from_argno(argno);

  if (mem_size > S32_MAX) {
          verbose(env, "R%d memory size %u is too large\n",
                  regno, mem_size);
          return -EACCES;
  }

be the intended form, or should this switch to reg_arg_name(env, argno)
with a matching update to the selftest __msg()?

> diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> index dc09d0e2d8ad..75a2e3f48d0f 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> @@ -152,6 +152,23 @@ int anon_user_mem_valid(void *ctx)
>  	return subprog_user_anon_mem(&t);
>  }
>
> +__noinline __weak int subprog_user_anon_mem_huge(int (*p)[0x3fffffff])
> +{
> +	return p ? (*p)[1] : 0;
> +}
> +
> +SEC("?tracepoint")
> +__failure __log_level(2)
> +__msg("R1 memory size 4294967292 is too large")
> +int anon_user_mem_huge_size_invalid(void *ctx)
> +{
> +	int (*p)[0x3fffffff];
> +	int tiny = 42;
> +
> +	p = (void *)&tiny;
> +	return subprog_user_anon_mem_huge(p) + tiny;
> +}
> +


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26556945758