Re: [PATCH bpf-next v2 04/35] bpf: Add join of registers and stack slots

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: join_reg() zeroes dst->delta under explore_alu_limits, failing regsafe()
--

commit 0a4e5dc6a51300431e525f23f800662f55dec8e9
Author: Alexei Starovoitov <ast@kernel.org>

bpf: Add join of registers and stack slots

This patch adds the first part of verifier state joining, covering ID
bijection tracking, register ranges, stack slots, and loop linear-register
detection for widening.

> diff --git a/kernel/bpf/join.c b/kernel/bpf/join.c
> new file mode 100644
> index 0000000000000..fe513b7a8e884
> --- /dev/null
> +++ b/kernel/bpf/join.c

[ ... ]

> +static bool join_reg(struct bpf_join_state *js, struct bpf_reg_state *dst,
> +		     const struct bpf_reg_state *ra, const struct bpf_reg_state *rb, int pass)
> +{
> +	if (ra->type == NOT_INIT || rb->type == NOT_INIT) {
> +		if (pass == 2 && ra->type != NOT_INIT)
> +			*dst = *rb;
> +		return true;
> +	}
> +
> +	if (ra->type != rb->type)
> +		return false;
> +
> +	switch (base_type(ra->type)) {
> +	case SCALAR_VALUE:
> +		if (js->env->explore_alu_limits && !reg_value_equal(ra, rb))
> +			return false;
> +		if (pass == 1) {
> +			jid_note(js, ra->id, rb->id, scalar_link_consistent(ra, rb));
> +			return true;
> +		}
> +		join_range(js, dst, ra, rb);
> +		/* a link that exists on one side only is dropped */
> +		if (ra->id && !jid_good(js, ra->id, rb->id)) {
> +			dst->id = 0;
> +			dst->delta = 0;
> +		}
> +		return true;

[Severity: Medium]
Can zeroing dst->delta here cause bpf_state_subsumes() -> states_equal() ->
regsafe() to fail when js->env->explore_alu_limits is true?

If two states have a SCALAR_VALUE register where both ra and rb have
BPF_ADD_CONST set with identical values and the same non-zero delta,
reg_value_equal(ra, rb) succeeds. However, if their base IDs do not form a
valid bijection across the states, jid_good(js, ra->id, rb->id) returns
false in pass 2 and join_reg() sets dst->id = 0 and dst->delta = 0.

When the result is subsequently checked by regsafe() in
kernel/bpf/states.c:

	case SCALAR_VALUE:
		if (env->explore_alu_limits) {
			return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
			       check_scalar_ids(rold->id, rcur->id, idmap);
		}

Because delta is placed before id in struct bpf_reg_state:

struct bpf_reg_state {
	enum bpf_reg_type type;
	s32 delta;
	...
	u32 id;

memcmp() compares dst->delta (0) against ra->delta (non-zero) and returns
false, causing the subsumption check on the joined state to fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105204513.2230344-1-alexei.starovoitov@gmail.com?part=4