[PATCH bpf-next 2/5] bpf: Keep the barrier of a nospec insn when removing nops

[Qiliang Yuan]

When an insn is unsafe on a mispredicted path, the verifier marks it
nospec, and bpf_convert_ctx_accesses() puts a barrier in front of it.
The verifier doesn't check the mispredicted path beyond the insn.

bpf_opt_remove_nops() runs before that and doesn't look at the mark. A
conditional jump can be nospec, then become a 'goto' over dead code,
and then a nop. The pass removes it, and the barrier goes with it. Here
the jump behind 'r1 = 0' compared r1 = r10 on the mispredicted path, for
a prog loaded with CAP_BPF and without CAP_PERFMON:

  0: call unknown
  1: r0 &= 255
  2: r1 = 0
  3: r0 = 0
  4: exit

Don't take a nospec insn for a nop.

Fixes: d6f1c85f2253 ("bpf: Fall back to nospec for Spectre v1")
Cc: stable@xxxxxxxxxxxxxxx # 6.17+
Reported-by: Alexei Starovoitov <ast@xxxxxxxxxx>
Closes: https://lore.kernel.org/bpf/DMKHZS5B5CNO.186F6YH1T6YU4@xxxxxxxxx/
Signed-off-by: Qiliang Yuan <odys.yuan@xxxxxxxxx>
---
For stable: before the patch list, opt_remove_nops() in verifier.c
removes 'ja 0' and 'may_goto +0'. Skip an insn whose aux data has nospec
there.
---
 kernel/bpf/fixups.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index ead78713e04f4..0cc758749be9b 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -699,7 +699,8 @@ static bool insn_is_nop(struct bpf_verifier_env *env, int i, int next)
 {
 	struct bpf_insn insn = env->prog->insnsi[i];
 
-	if (env->insn_aux_data[i].indirect_target)
+	/* keep a nospec insn, the barrier in front of it must stay */
+	if (env->insn_aux_data[i].indirect_target || env->insn_aux_data[i].nospec)
 		return false;
 	if (insn.off < 0 || i + 1 + insn.off > next)
 		return false;

-- 
2.43.0