Re: [PATCH bpf-next v2 1/8] bpf: mark instructions accessing program stack

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Patched instructions incorrectly lose their `non_stack_access` marking, causing JITs to erroneously skip KASAN instrumentation for actual memory accesses.
- [Medium] Out-of-bounds pointer arithmetic in `bpf_insn_accesses_stack_only` when JIT compiling eBPF subprograms.
--

commit ae20410c7f06315eaa729fac3ff9e9abfe3062e2
Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

bpf: mark instructions accessing program stack

This patch introduces functionality to mark BPF instructions that access the
program stack. The verifier tracks non-stack memory accesses, allowing JIT
compilers to skip emitting KASAN instrumentation for stack accesses while
enforcing it for other memory types.

> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index a656a8572bdbe..393d9eacd2156 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1583,6 +1583,22 @@ bool bpf_insn_is_indirect_target(const struct bpf_verifier_env *env, const struc
>  	return env->insn_aux_data[insn_idx].indirect_target;
>  }
>  
> +bool bpf_insn_accesses_stack_only(const struct bpf_verifier_env *env,
> +				  const struct bpf_prog *prog, int insn_idx)
> +{
> +	struct bpf_insn *insn;
> +
> +	/* cBPF: we have no verifier state, do a best-effort check based on
> +	 * dst/src reg
> +	 */
> +	insn_idx += prog->aux->subprog_start;
> +	insn = (struct bpf_insn *)prog->insnsi + insn_idx;
> +	if (!env)

[Severity: Medium]
When JIT compiling an eBPF subprogram, prog points to the subprogram and
prog->insnsi only spans the subprogram's instructions. By adding
prog->aux->subprog_start to insn_idx, doesn't this result in an
out-of-bounds pointer arithmetic for insn?

Even though the pointer is not dereferenced here because of the if (!env)
check, could this trigger UBSAN bounds-checking traps? Should the insn
pointer calculation be deferred until inside the !env block where it is
genuinely needed for cBPF?

> +		return insn->dst_reg == BPF_REG_FP ||
> +		       insn->src_reg == BPF_REG_FP;
> +	return !env->insn_aux_data[insn_idx].non_stack_access;
> +}
> +

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 5aa3f7d99ac92..5228c910fbf56 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
[ ... ]
> @@ -185,16 +185,22 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env,
[ ... ]
>  	if (data[off + cnt - 1].indirect_target) {
>  		data[off].indirect_target = 1;
>  		data[off + cnt - 1].indirect_target = 0;
>  	}
> +
> +	if (data[off + cnt - 1].non_stack_access) {
> +		data[off].non_stack_access = 1;
> +		data[off + cnt - 1].non_stack_access = 0;
> +	}

[Severity: High]
When bpf_patch_insn_data() expands a single instruction into multiple
instructions, this logic restricts the non_stack_access flag to only the
first instruction of the patched sequence and clears it for the rest.

Since the actual memory load or store in a patched sequence often occurs on
subsequent instructions rather than the first, wouldn't they lose this flag
and incorrectly bypass KASAN checks during JIT compilation?

>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-kasan-v2-0-c066e627fda8@bootlin.com?part=1