Re: [PATCH bpf 1/2] bpf: Fix stack slot index in nospec checks

[Luis Gerhorst]

Nuoqi Gui <gnq25@xxxxxxxxxxxxxxxxxxxxx> writes:

> check_stack_write_fixed_off() computes the byte slot for a fixed-offset
> stack write as -off - 1, and records each written byte in slot_type[] with
> (slot - i) % BPF_REG_SIZE.
>
> The Spectre v4 sanitization pre-check uses slot_type[i] instead. For a
> 4-byte write at fp-8 after the lower half of fp-8 has been zeroed, the
> pre-check scans bytes 0..3 and sees STACK_ZERO while the actual write updates
> bytes 7..4. That can leave the second half-slot write without nospec_result
> even though the bytes being overwritten still require sanitization.
>
> Use the same slot index in the sanitization pre-check that the write path uses
> when updating slot_type[].
>
> Fixes: e4f4db47794c ("bpf: Fix pointer-leak due to insufficient speculative store bypass mitigation")
> Signed-off-by: Nuoqi Gui <gnq25@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  kernel/bpf/verifier.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2abc79dbf281c..50e80dbbc1784 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3479,7 +3479,8 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
>  		bool sanitize = reg && is_spillable_regtype(reg->type);
>  
>  		for (i = 0; i < size; i++) {
> -			u8 type = state->stack[spi].slot_type[i];
> +			u8 type = state->stack[spi].slot_type[(slot - i) %
> +							      BPF_REG_SIZE];
>  
>  			if (type != STACK_MISC && type != STACK_ZERO) {
>  				sanitize = true;

Acked-by: Luis Gerhorst <luis.gerhorst@xxxxxx>

I have briefly checked the other uses of slot_type[i] and they look
fine.