Re: [PATCH bpf-next 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers()

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Critical verifier bug in conditional jump liveness tracking causing missing zero-extensions.
- [Low] Suboptimal liveness tracking for 32-bit conditional jumps causes unnecessary zero-extensions.
--

--- Patch [2]: [PATCH bpf-next 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() ---
commit 977deff7b46d8e36a957595add75da7e58a91c59
Author: Eduard Zingerman <[email protected]>

bpf: track upper 32-bit register halves' liveness in compute_live_registers()

This commit extends compute_live_registers() to track upper and lower
register halves' liveness separately. It uses 2 bits per register and
updates compute_insn_live_regs() and kfunc call processing to infer usage
based on BTF type sizes.

> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index ff1e68cc4bd1c..c996b8a03e792 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c
[ ... ]
> @@ -2174,17 +2228,15 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env,
[ ... ]
>  		default:
>  			def = 0;
>  			if (BPF_SRC(insn->code) == BPF_K)
>  				use = dst;

[Severity: Low]
Does this unconditionally apply the 64-bit mask for 32-bit jumps?

Because dst is defined as the 64-bit mask, using it for BPF_JMP32
conditional jumps incorrectly assumes the upper 32 bits are alive.

Could this trigger unnecessary zero-extension instructions in the JIT?

>  			else
> -				use = dst | src;
> +				use = dst | (code == BPF_JMP32 ? src : src32);

[Severity: Critical]
Are the conditions and logic for tracking 64-bit jumps correct here?

The variable code is derived using BPF_OP(), which masks out the class bits.
Because of this, it will never equal the instruction class BPF_JMP32,
making the condition always false.

Additionally, if this were checking the class instead of code, it appears
the branches of the ternary operator are reversed. As written, 64-bit jumps
always evaluate to src32, marking only the lower 32 bits of the source
register as live.

Could this cause the verifier to incorrectly omit the zero-extension flag
for the preceding instruction that produced the source register, potentially
leaving garbage data in the upper 32 bits during 64-bit comparisons?

>  		}
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2