Thread (18 messages) 18 messages, 3 authors, 2025-03-19

Re: [PATCH bpf-next 11/11] bpf: Fall back to nospec for spec path verification

From: Alexei Starovoitov <hidden>
Date: 2025-03-19 02:40:57
Also in: bpf, linux-arm-kernel, linux-kselftest, lkml

On Thu, Mar 13, 2025 at 10:57 AM Luis Gerhorst [off-list ref] wrote:
This trades verification complexity for runtime overheads due to the
nospec inserted because of the EINVAL.

With increased limits this allows applying mitigations to large BPF
progs such as the Parca Continuous Profiler's prog. However, this
requires a jump-seq limit of 256k. In any case, the same principle
should apply to smaller programs therefore include it even if the limit
stays at 8k for now. Most programs in [1] only require a limit of 32k.
Do you mean that without this change the verifier needs 256k
jmp limit to load Parca's prog as unpriv due to speculative
path exploration with push_stack ?

And this change uses 4k as a trade-off between prog runtime
and verification time ?

But tracing progs use bpf_probe_read_kernel(), so they're never going
to be unpriv.
quoted hunk ↗ jump to hunk
@@ -2010,6 +2011,19 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env,
        struct bpf_verifier_stack_elem *elem;
        int err;

+       if (!env->bypass_spec_v1 &&
+           cur->speculative &&
Should this be
(cur->speculative || speculative)
?

In general I'm not convinced that the approach is safe.

This recoverable EINVAL means that exploration under speculation
stops early, but there could be more branches and they won't be
sanitized with extra lfence.
So speculative execution can still happen at later insns.

Similar concern in patch 7:
+ if (state->speculative && cur_aux(env)->nospec)
+   goto process_bpf_exit;

One lfence at this insn doesn't stop speculation until the program end.
Only at this insn. The rest of the code is free to speculate.

The refactoring in patches 1-3 is nice.
Patches 4-5 are tricky and somewhat questionable, but make sense.
Patch 7 without early goto process_bpf_exit looks correct too,
Patch 8 is borderline. Feels like it's opening the door for
new vulnerabilities and space to explore for security researchers.
We disabled unpriv bpf by default and have no intentions to enable it.
Even if we land the whole thing the unpriv will stay disabled.
So trade offs don't appear favorable.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help