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

[Yonghong Song <yonghong.song@linux.dev>]

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. */
> [...]