Re: [PATCH bpf-next 6/7] bpf: preserve constant zero when doing partial register restore

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2023-10-30 at 22:03 -0700, Andrii Nakryiko wrote:
> Similar to special handling of STACK_ZERO, when reading 1/2/4 bytes from
> stack from slot that has register spilled into it and that register has
> a constant value zero, preserve that zero and mark spilled register as
> precise for that. This makes spilled const zero register and STACK_ZERO
> cases equivalent in their behavior.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Could you please add a test case?

[...]

> ---
>  kernel/bpf/verifier.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0eecc6b3109c..8cfe060e4938 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4958,22 +4958,39 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
>  				copy_register_state(&state->regs[dst_regno], reg);
>  				state->regs[dst_regno].subreg_def = subreg_def;
>  			} else {
[...]
> +
> +				if (spill_cnt == size &&
> +				    tnum_is_const(reg->var_off) && reg->var_off.value == 0) {
> +					__mark_reg_const_zero(&state->regs[dst_regno]);
> +					/* this IS register fill, so keep insn_flags */
> +				} else if (zero_cnt == size) {
> +					/* similarly to mark_reg_stack_read(), preserve zeroes */
> +					__mark_reg_const_zero(&state->regs[dst_regno]);
> +					insn_flags = 0; /* not restoring original register state */
> +				} else {
> +					mark_reg_unknown(env, state->regs, dst_regno);
> +					insn_flags = 0; /* not restoring original register state */
> +				}

Condition for this branch is (off % BPF_REG_SIZE != 0) || size != spill_size,
is it necessary to check for some unusual offsets, e.g. off % BPF_REG_SIZE == 7
or something like that?

[...]