Re: [PATCH bpf-next v3 1/2] bpf: Track aligned st store as imprecise spilled registers

[Yonghong Song <yonghong.song@xxxxxxxxx>]

On 1/9/24 10:02 AM, Eduard Zingerman wrote:
> On Mon, 2024-01-08 at 20:05 -0800, Yonghong Song wrote:
> [...]
> > @@ -4640,7 +4641,18 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
> >   			return -EINVAL;
> >   		}
> >   
> > -		/* Erase all spilled pointers. */
> > +		/* If writing_zero and the the spi slot contains a spill of value 0,
> > +		 * maintain the spill type.
> > +		 */
> > +		if (writing_zero && is_spilled_scalar_reg(&state->stack[spi])) {
> As discussed on offlist today, this should probably look as follows:
>
> -               if (writing_zero && is_spilled_scalar_reg(&state->stack[spi])) {
> +               if (writing_zero && *stype == STACK_SPILL && is_spilled_scalar_reg(&state->stack[spi])) {
>
> In order to handle cases like "mmmmSSSS" for slot types.

Thanks Eduard for pointing this out. I did handle this in v2 but missed it in v3.
I will wait a little bit just in case there are some other comments before posting v4
to fix this issue.

> > +			spill_reg = &state->stack[spi].spilled_ptr;
> > +			if (tnum_is_const(spill_reg->var_off) && spill_reg->var_off.value == 0) {
> > +				zero_used = true;
> > +				continue;
> > +			}
> > +		}
> > +
> > +		/* Erase all other spilled pointers. */
> >   		state->stack[spi].spilled_ptr.type = NOT_INIT;
> >   
> >   		/* Update the slot type. */
> [...]