Re: [PATCH v3 bpf-next 13/18] bpf: Check the return values of fmod_ret programs on found targets

[bot+bpf-ci@kernel.org]

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

> bpf: Check the return values of fmod_ret programs on found targets

TL;DR: Possible wrong behaviour: fmod_ret programs on 64-bit found targets
don't require the JIT, so in the interpreter -14 may reach callers as
4294967282; a later commit fixes this, leaving a narrow gap here.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> +static int fmodret_retval_init(struct bpf_verifier_env *env, u32 btf_id,
> +			       unsigned long addr, const char *tname)
> +{

[ ... ]

> +	if (!cnt)
> +		goto invalid;
> +
> +	aux->fmodret_retvals = retvals;
> +	aux->fmodret_retval_cnt = cnt;
> +	return 0;

For a found target that returns a 64-bit integer (size == 8),
check_fmodret_retval() accepts the full 64-bit R0, so "r0 = -14" is
accepted as the s64 value -14.  Nothing here makes such a program require
the JIT.

If the program runs in the interpreter, prog->bpf_func is
__bpf_prog_runN(), which is declared in kernel/bpf/core.c as:

  static unsigned int PROG_NAME(stack_size)(const void *ctx,
                                            const struct bpf_insn *insn)

so only the low 32 bits of R0 come back.  The x86 trampoline handles
interpreted programs, stores all of RAX and returns it to the caller of
the traced function, so the caller of a long-returning found target would
see 4294967282 instead of -14.

That value is non-zero, so the function body is skipped, but it is not an
error value either.  Callers would treat the skipped call as a success
that returned a large positive value, which is the case the commit message
says the check is meant to prevent ("Returning such a value instead of
calling the function looks to the callers like a real failure").

Should fmodret_retval_init() set env->prog->jit_required = 1 when
size == 8?  A later commit in this series, c4a7eee0c4d7 ("bpf: Require the
JIT for fmod_ret programs on found 64-bit targets"), adds
"if (size == 8) env->prog->jit_required = 1;" right after these
assignments.  No found target carries the tag until 8659bd72073c
("resolve_btfids: Tag fmod_ret candidates as targets"), which comes after
that fix, so the window is narrow, but it is still a gap in this commit's
own logic.


---
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/42981885558