Insert a nospec before the access to prevent it from ever using an index
that is subject to speculative scalar-confusion.

The access itself can either happen directly in the BPF program (reads
only, check_stack_read_var_off()) or in a helper (read/write,
check_helper_mem_access()).

This relies on the fact that the speculative scalar confusion that leads
to the variable-stack access going OOBs must stem from a prior
speculative store or branch bypass. Adding a nospec before the
variable-stack access will force all previously bypassed stores/branches
to complete and cause the stack access to only ever go to the stack slot
that is accessed architecturally.

Alternatively, the variable-offset stack access might be a write that
can itself be subject to speculative store bypass (this can happen in
theory even if this code adds a nospec /before/ the variable-offset
write). Only indirect writes by helpers might be affected here (e.g.,
those taking ARG_PTR_TO_MAP_VALUE). (Because check_stack_write_var_off()
does not use check_stack_range_initialized(), in-program variable-offset
writes are not affected.) If the in-helper write can be subject to
Spectre v4 and the helper writes/overwrites pointers on the BPF stack,
they are already a problem for fixed-offset stack accesses and should be
subject to Spectre v4 sanitization.

Signed-off-by: Luis Gerhorst <redacted>
Acked-by: Henriette Herzog <redacted>
Cc: Maximilian Ott <redacted>
Cc: Milan Stephan <redacted>
---
 kernel/bpf/verifier.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index db26b477dd45..1fbafea3ed69 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c

@@ -7894,6 +7894,11 @@ static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn)
        }
 }

+static struct bpf_insn_aux_data *cur_aux(const struct bpf_verifier_env *env)
+{
+       return &env->insn_aux_data[env->insn_idx];
+}
+
 /* When register 'regno' is used to read the stack (either directly or through
  * a helper function) make sure that it's within stack boundary and, depending
  * on the access type and privileges, that all elements of the stack are

@@ -7933,18 +7938,18 @@ static int check_stack_range_initialized(
        if (tnum_is_const(reg->var_off)) {
                min_off = max_off = reg->var_off.value + off;
        } else {
-               /* Variable offset is prohibited for unprivileged mode for
+               /* Variable offset requires a nospec for unprivileged mode for
                 * simplicity since it requires corresponding support in
                 * Spectre masking for stack ALU.
                 * See also retrieve_ptr_limit().
                 */
                if (!env->bypass_spec_v1) {
-                       char tn_buf[48];
-
-                       tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
-                       verbose(env, "R%d variable offset stack access prohibited for !root, var_off=%s\n",
-                               regno, tn_buf);
-                       return -EACCES;
+                       /* Allow the access, but prevent it from using a
+                        * speculative offset using a nospec before the
+                        * dereference op.
+                        */
+                       cur_aux(env)->nospec = true;
+                       WARN_ON_ONCE(cur_aux(env)->alu_state);
                }
                /* Only initialized buffer on stack is allowed to be accessed
                 * with variable offset. With uninitialized buffer it's hard to

@@ -11172,11 +11177,6 @@ static int check_get_func_ip(struct bpf_verifier_env *env)
        return -ENOTSUPP;
 }