SSB extra 0

BPF is a potential source of gadgets that can be used for memory diambiguation-based attacks. To help mitigate these, we enable the bit in SPEC_CTRL which enables the reduced (memory) speculation mode on the processor when runing BPF code.

Quoted 4 lines

BPF is a potential source of gadgets that can be used for memory diambiguation-based attacks. To help mitigate these, we enable the bit in SPEC_CTRL which enables the reduced (memory) speculation mode on the processor when runing BPF code.

Do you mean eBPF, or even cBPF?

For example, can gadgets be built using the BPF used in seccomp()? Prior speculation flaws weren't exposed there, so it might be possible (though ironic given my other seccomp series) to not trigger this for seccomp BPF execution... :P

BPF code is often supplied from outside the kernel. While it can be programmatically verified, it is very difficult to verify potential effects from speculative execution.

This patch adds some marker functions as BFP code is entered or exited. These serve only a stubs for now.

There are many possibilities for optimization. The BFP programs that run on devices, for instance, are less likely to need any CPU-based mitigations. These patches are an entirely unoptimized first pass.

Review int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array, · line 365

Quoted 10 lines

(quoted diff) +2 -0

@@ -357,7 +357,9 @@ int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
 			goto _out;			\
 		_prog = _array->progs;			\
 		while ((__prog = READ_ONCE(*_prog))) {	\
+			bpf_enter_prog(__prog);		\
 			_ret &= func(__prog, ctx);	\
+			bpf_leave_prog(__prog);		\
 			_prog++;			\
 		}					\
 _out:							\

So I actually had a look at this yesterday, and why did you choose to put these hooks at the prog callsites instead of inside the prog itself?

It seems to me that placing them inside the program (either the interpreter or the jit) is much less invasive.

Review

Quoted 11 lines · speck for Dave Hansen

static void bpf_done_on_this_cpu(struct work_struct *work) {

(quoted diff) +7 -2

-	if (!this_cpu_dec_return(bpf_prog_ran))
-		return;
+	if (this_cpu_dec_return(bpf_prog_ran)) {
+		/*
+		 * This is unexpected.  The elevated refcount indicates
+		 * being in the *middle* of a BPF program, which should
+		 * be impossible.  They are executed inside
+		 * rcu_read_lock() where we can not sleep and where
+		 * preemption is disabled.

Preemptible RCU exists in mainline since 2008, so your assumption that preemption is disabled is simply wrong. And please don't just slap a preempt_disable() somewhere as this really can be done without. It's per thread information not per cpu.

Quoted 17 lines · speck for Thomas Gleixner

Review

Quoted 11 lines · speck for Dave Hansen

static void bpf_done_on_this_cpu(struct work_struct *work) {

(quoted diff) +7 -2

-	if (!this_cpu_dec_return(bpf_prog_ran))
-		return;
+	if (this_cpu_dec_return(bpf_prog_ran)) {
+		/*
+		 * This is unexpected.  The elevated refcount indicates
+		 * being in the *middle* of a BPF program, which should
+		 * be impossible.  They are executed inside
+		 * rcu_read_lock() where we can not sleep and where
+		 * preemption is disabled.

Preemptible RCU exists in mainline since 2008, so your assumption that preemption is disabled is simply wrong. And please don't just slap a preempt_disable() somewhere as this really can be done without. It's per thread information not per cpu.

Ahhh, that's an interesting point. Are you thinking that we add a BPF "instruction" to enable or disable the mitigations, and then have the verifier insert it as the first instruction and then before any exit instructions? Is there some precedent for doing this?

Quoted 13 lines · speck for Peter Zijlstra

Review int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array, · line 365

Quoted 10 lines

(quoted diff) +2 -0

@@ -357,7 +357,9 @@ int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
 			goto _out;			\
 		_prog = _array->progs;			\
 		while ((__prog = READ_ONCE(*_prog))) {	\
+			bpf_enter_prog(__prog);		\
 			_ret &= func(__prog, ctx);	\
+			bpf_leave_prog(__prog);		\
 			_prog++;			\
 		}					\
 _out:							\

So I actually had a look at this yesterday, and why did you choose to put these hooks at the prog callsites instead of inside the prog itself?

Quoted 2 lines

It seems to me that placing them inside the program (either the interpreter or the jit) is much less invasive.

It's not a bad idea. This didn't seem *too* invasive other than I had to hit all the variants of calling func().

Were you thinking we add some eBPF instruction which calls out to the kernel machinery and then the kernel BPF code inserts it as the first instruction of the program?

Teaching the interpreter to do this is trivial, for the JIT we would emit a "CALL $asm_pre_thunk" as first instruction and "CALL $asm_post_thunk" before every RET.

And when it doesn't need to mitigate, don't issue the instructions at all.

Quoted 4 lines

Ahhh, that's an interesting point. Are you thinking that we add a BPF "instruction" to enable or disable the mitigations, and then have the verifier insert it as the first instruction and then before any exit instructions? Is there some precedent for doing this?

Dunno, but it would be the obvious thing to do I think.

Quoted 6 lines

Quoted 4 lines

Ahhh, that's an interesting point. Are you thinking that we add a BPF "instruction" to enable or disable the mitigations, and then have the verifier insert it as the first instruction and then before any exit instructions? Is there some precedent for doing this?

Dunno, but it would be the obvious thing to do I think.

Other option would be a preempt notifier migrating the SSB state?

Quoted 5 lines · speck for Andi Kleen

Quoted 5 lines

Quoted 4 lines

Ahhh, that's an interesting point. Are you thinking that we add a BPF "instruction" to enable or disable the mitigations, and then have the verifier insert it as the first instruction and then before any exit instructions? Is there some precedent for doing this?

Quoted 3 lines

Quoted 1 line

Dunno, but it would be the obvious thing to do I think.

Other option would be a preempt notifier migrating the SSB state?

Why so? Because it's more complex than just using the TIF_RDS flag which gets evaluated anyway?

Quoted 9 lines · speck for Thomas Gleixner

Quoted 7 lines · speck for Andi Kleen

Quoted 6 lines

Quoted 4 lines

Ahhh, that's an interesting point. Are you thinking that we add a BPF "instruction" to enable or disable the mitigations, and then have the verifier insert it as the first instruction and then before any exit instructions? Is there some precedent for doing this?

Dunno, but it would be the obvious thing to do I think.

Quoted 4 lines

Quoted 1 line

Other option would be a preempt notifier migrating the SSB state?

Why so? Because it's more complex than just using the TIF_RDS flag which gets evaluated anyway?

The flag doesn't know anything about the timer. You would need another flag that says "start a delay timer on the new CPU too".

Quoted 3 lines

The flag doesn't know anything about the timer. You would need another flag that says "start a delay timer on the new CPU too".

Color me confused. I dont see a timer anywhere.

From: Dave Hansen <[email protected]>
Subject: x86, bugs: centralize SPEC_CTRL MSR mask generation

From: Dave Hansen <[email protected]>

The KVM code manipualtes the SPEC_CTRL MSR when it enters and exits the guest. It overwrites the "kernel" value when it enters the guest and restores the "kernel" value after leaving the guest.

Both code paths take into account the "base" (x86_spec_ctrl_base) value and the per-task TIF_RDS flag (on Intel). They then see if the new state differs from the existing state and avoid the MSR write if no change is made.

But, these two paths could be a bit more unified. Introduce a new
function: x86_calculate_kernel_spec_ctrl() which will figure out the "kernel" value to contrast it with the "guest" value. We also rename the arguments to the set/restore functions to make it clear that while the arguments are both "guest" state, they really mean different things to the two functions.

This will make the next step easier when we have more state to consult in doing the x86_calculate_kernel_spec_ctrl() calculatione

Signed-off-by Dave Hansen <[email protected]>

Quoted 4 lines · speck for Thomas Gleixner

Quoted 3 lines

The flag doesn't know anything about the timer. You would need another flag that says "start a delay timer on the new CPU too".

Color me confused. I dont see a timer anywhere.

We were trying not to ping-pong the MSRs too much if we are going in/out of the BFP code frequently.

So, there's a schedule_delayed_work_on() in there that waits 10ms and turns the mitigations off, so we're only ping-ponging the MSRs every 10ms.

So, even in the case that we're generating BPF instructions for the mitigation enable/disable, we still might want some mechanism to make sure that we're not touching the MSRs *too* frequently if we're going in/out of BFP frequently.

That's a wee bit harder if we're tracking the mitigation on the task level than the CPU. I don't think it's impossible, but it's certainly more code than is there at the moment.

Quoted 5 lines · speck for Dave Hansen

Quoted 4 lines

BPF is a potential source of gadgets that can be used for memory diambiguation-based attacks. To help mitigate these, we enable the bit in SPEC_CTRL which enables the reduced (memory) speculation mode on the processor when runing BPF code.

Quoted 1 line

Do you mean eBPF, or even cBPF?

Right or wrong, my assumption is that you can build gadgets with any of the variants. I haven't looked into detail to whether the classic VM and enhanced VM have the building blocks.
[...]
Were we just not concerned before because seccomp didn't use arrays?

[...]
Either way, I don't think we have the BPF experts on this list, yet. I'm a BPF newbie for sure.

Now lets look at EBPF. EBPF is also a nesting context.

So, if EBPF runs in preemptible task context, then it sets a flag in the task 'ebf_speculation_disabled' and sets TIF_RDS, which means that on migration the normal switch_to() logic will take care of it. Obviously we need a per task storage for the prctl selected state. I already did this for the force disable thing. If EBF reenables then it uses the per task prctl state.

If EBPF runs in soft or hardirq context then it can uses the per cpu refcount. The above rules apply.

Quoted 11 lines · speck for Thomas Gleixner

Now lets look at EBPF. EBPF is also a nesting context.

So, if EBPF runs in preemptible task context, then it sets a flag in the task 'ebf_speculation_disabled' and sets TIF_RDS, which means that on migration the normal switch_to() logic will take care of it. Obviously we need a per task storage for the prctl selected state. I already did this for the force disable thing. If EBF reenables then it uses the per task prctl state.

If EBPF runs in soft or hardirq context then it can uses the per cpu refcount. The above rules apply.

Following up the discussion on IRC:

- BPF runs also in NMI context, but that's not a real problem

- BPF programms can 'call' other BPF programs

There are two forms: Regular calls which return to the caller and tail calls. Tail calls make the speculation control more complicated because the resulting disable/enable invocations become imbalanced.

So the counter does not work, but we can makee BPF speculation control to use a simple boolean flag which denotes that speculation has been disabled.

The JIT or interpreter injects only the speculation_disable() call into the BPF programs which are not trusted, but no speculation_enable() call.

speculation_enable() is called unconditionally at the BPF invocation wrapper after the BPF execution terminates.

speculation_disable() notes the disabling per context (thread, softirq, hardirq, nmi) and speculation_enable() checks the disabled bit for the context and acts accordinlgy.

That means for trusted programs which do not disable speculation speculation_enable() is a simple bit check and return.

If in the call chain of a BPF program speculation is disabled then it stays disabled unti the program returns.

1) I read about trusted and untrusted BPF programs. How is that determined/defined?

Quoted 2 lines · speck for Thomas Gleixner

1) I read about trusted and untrusted BPF programs. How is that determined/defined?

There's a rather steep learning curve, I fear.

There's bpf_map->unpriv_array which tells whether or not you have to deploy Spectre V1 mitigations. That's just derived from CAP_SYS_ADMIN when the map is allocated.

Separately, I've seen some hard-coded checks against BPF_PROG_TYPE_*, like may_access_direct_pkt_data().

So I think the trust ends up being anchored in the interfaces that allocated the program (or map?) to begin with, but it's somewhat hard to follow.

Is that what you meant?

arch/x86/kernel/cpu/bugs.c +5 -1

index 0e17379..947ae8a 100644
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/utsname.h>
 #include <linux/cpu.h>
+#include <linux/filter.h>
 #include <linux/module.h>
 #include <linux/nospec.h>
 #include <linux/prctl.h>

@@ -148,10 +149,23 @@ EXPORT_SYMBOL_GPL(x86_get_default_spec_ctrl);
 
 static inline u64 intel_rds_mask(void)
 {
+	u64 mask;
+
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return 0;
 
-	return rds_tif_to_spec_ctrl(current_thread_info()->flags);
+	mask = rds_tif_to_spec_ctrl(current_thread_info()->flags);
+

Quoted 21 lines · speck for Dave Hansen

arch/x86/kernel/cpu/bugs.c +3 -0

index 0e17379..947ae8a 100644
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/utsname.h>
 #include <linux/cpu.h>
+#include <linux/filter.h>
 #include <linux/module.h>
 #include <linux/nospec.h>
 #include <linux/prctl.h>

@@ -148,10 +149,23 @@ EXPORT_SYMBOL_GPL(x86_get_default_spec_ctrl);
 
 static inline u64 intel_rds_mask(void)
 {
+	u64 mask;
+
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return 0;
 

arch/x86/kernel/cpu/bugs.c EXPORT_SYMBOL_GPL(x86_get_default_spec_ctrl); · line 168

Quoted 13 lines

arch/x86/kernel/cpu/bugs.c +12 -1

-	return rds_tif_to_spec_ctrl(current_thread_info()->flags);
+	mask = rds_tif_to_spec_ctrl(current_thread_info()->flags);
+
+	/*
+	 * BPF programs can be exploited to attack the kernel.
+	 * Leave the RDS bit on when we recently ran one.  This
+	 * bit gets cleared after a BFP program has not run on
+	 * the CPU for a while.
+	 */
+	if (get_cpu_var(bpf_prog_ran))
+		mask |= SPEC_CTRL_RDS;
+
+	return mask;

I know that different aproach is being taken for BPF mitigation, but in case anyone is basing his first wave of updates on this ...

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

BPF is a potential source of gadgets that can be used for memory=20 diambiguation-based attacks. To help mitigate these, we enable the bit in SPEC_CTRL which enables the reduced (memory) speculation mode on the processor when runing BPF code.

Quoted 8 lines

Quoted 2 lines

=20
Do you mean eBPF, or even cBPF?=20

=20
Right or wrong, my assumption is that you can build gadgets with any of the variants. I haven't looked into detail to whether the classic VM and enhanced VM have the building blocks.
[...]
Were we just not concerned before because seccomp didn't use arrays?

My understanding from Jann Horn (who looked at the seccomp cases before) was that eBPF maps were required for SpectreV1. I've asked him for clarification on SSB, but I think the reasoning holds there too.

Specifically, my memory of the details are that since seccomp uses a subset of even cBPF and can only use absolute indexing (not dynamic), cBPF programs cannot be constructed that do array access outside of the seccomp data buffer (which consists entirely of userspace information: pid, syscall number, syscall arguments), not even from the 16-byte "scratch" space. i.e. the seccomp cBPF verifier, seccomp_check_filter(), results in no seccomp cBPF programs having the "untrusted_offset_from_caller" indexing (to borrow the variable name from the Project Zero write-up on SpectreV1). It also has no maps (so there cannot be data sharing between multiple seccomp cBPF programs nor to the outside world). And finally, the only "visible" output from seccomp is via syscall behaviors, which would create too much noise for that exfiltration path.

Note, though, that I am not an expert in eBPF nor writing speculation exploits. Which gets us to the next question...

Quoted 3 lines

[...]
Either way, I don't think we have the BPF experts on this list, yet. I'm a BPF newbie for sure.

What's the plan for involving them?

Whatever the case, I think we need to find a way to exempt seccomp cBPF =66rom this...

Quoted 30 lines · speck for Kees Cook

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

BPF is a potential source of gadgets that can be used for memory=20 diambiguation-based attacks. To help mitigate these, we enable the bit in SPEC_CTRL which enables the reduced (memory) speculation mode on the processor when runing BPF code.

Quoted 8 lines

Quoted 2 lines

=20
Do you mean eBPF, or even cBPF?=20

=20
Right or wrong, my assumption is that you can build gadgets with any of the variants. I haven't looked into detail to whether the classic VM and enhanced VM have the building blocks.
[...]
Were we just not concerned before because seccomp didn't use arrays?

My understanding from Jann Horn (who looked at the seccomp cases before) was that eBPF maps were required for SpectreV1. I've asked him for clarification on SSB, but I think the reasoning holds there too.

Specifically, my memory of the details are that since seccomp uses a subset of even cBPF and can only use absolute indexing (not dynamic), cBPF programs cannot be constructed that do array access outside of the seccomp data buffer (which consists entirely of userspace information: pid, syscall number, syscall arguments), not even from the 16-byte "scratch" space. i.e. the seccomp cBPF verifier, seccomp_check_filter(), results in no seccomp cBPF programs having the "untrusted_offset_from_caller" indexing (to borrow the variable name from the Project Zero write-up on SpectreV1). It also has no maps (so there cannot be data sharing between multiple seccomp cBPF programs nor to the outside world). And finally, the only "visible" output from seccomp is via syscall behaviors, which would create too much noise for that exfiltration path.

Quoted 2 lines

Note, though, that I am not an expert in eBPF nor writing speculation exploits. Which gets us to the next question...

Now that Alexei is on this list, let me bring the question of (seccomp-)cBPF up again.

Specifically I am of course asking to see whether we need any kind of mitigation in pre-eBPF (and also pre-1be7f75d1668 eBPF) kernels.