Re: [PATCH bpf 2/2] selftests/bpf: Cover stack nospec slot indexing

[Luis Gerhorst <luis.gerhorst@fau.de>]

Nuoqi Gui <gnq25@mails.tsinghua.edu.cn> writes:

> Add a verifier test for the fixed-offset stack write case where two 4-byte
> stores initialize opposite halves of the same stack slot.
>
> The test uses the CAP_BPF-without-CAP_PERFMON loader lane so Spectre v4
> mitigation remains active. It expects both half-slot writes to emit nospec
> in the translated program.
>
> Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
> ---
>  .../testing/selftests/bpf/progs/verifier_unpriv.c  | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_unpriv.c b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
> index c16f8382cf17d..9ebbd4b531df1 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_unpriv.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
> @@ -976,4 +976,27 @@ l0_%=:	exit;						\
>  	: __clobber_all);
>  }
>  
> +SEC("socket")
> +__description("noperfmon: Spectre v4 stack write slot index")
> +__success __success_unpriv
> +__caps_unpriv(CAP_BPF)

Not sure if the conditions that led Kartikeya and Eduard to add
__caps_unpriv(CAP_BPF) explicitly for some tests also apply here.

It seems adding it to this test will not provide much benefit because
the other tests in verifier_unpriv.c already assume
sysctl_unprivileged_bpf_disabled=0.

Because this already affects the more restricted 'unpriv process under
sysctl_unprivileged_bpf_disabled=0 without CAP_BPF' environment, I
suggest dropping __caps_unpriv(CAP_BPF) it.

> +__retval(0)
> +#ifdef SPEC_V4
> +__xlated_unpriv("r0 = 0")
> +__xlated_unpriv("*(u32 *)(r10 -4) = r0")
> +__xlated_unpriv("nospec")
> +__xlated_unpriv("*(u32 *)(r10 -8) = r0")
> +__xlated_unpriv("nospec")
> +__xlated_unpriv("exit")
> +#endif
> +__naked void stack_write_nospec_slot_index(void)
> +{
> +	asm volatile ("					\
> +	r0 = 0;						\
> +	*(u32 *)(r10 - 4) = r0;			\
> +	*(u32 *)(r10 - 8) = r0;			\
> +	exit;						\
> +"	::: __clobber_all);
> +}
> +
>  char _license[] SEC("license") = "GPL";

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