Unprivileged BPF default

Borrow CONFIG_CPU_SPECTRE from ARM to be available for all architectures. This will help in configuration of features that depend on CPU being affected by spectre class of vulnerabilities.

Signed-off-by Pawan Gupta <[email protected]>

---
 arch/Kconfig        | 3 +++
 arch/arm/mm/Kconfig | 3 ---
 arch/x86/Kconfig    | 1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

Borrow CONFIG_CPU_SPECTRE from ARM to be available for all architectures. This will help in configuration of features that depend on CPU being affected by spectre class of vulnerabilities.

Signed-off-by Pawan Gupta <[email protected]>

---
 arch/Kconfig        | 3 +++
 arch/arm/mm/Kconfig | 3 ---
 arch/x86/Kconfig    | 1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

arch/Kconfig +3 -0

index 8df1c7102643..6aa856d51cb7 100644
@@ -1091,6 +1091,9 @@ config ARCH_SUPPORTS_RT
 config CPU_NO_EFFICIENT_FFS
 	def_bool n
 
+config CPU_SPECTRE
+	bool
+
 config HAVE_ARCH_VMAP_STACK
 	def_bool n
 	help

arch/arm/mm/Kconfig +0 -3

index 8355c3895894..44551465fd03 100644
@@ -828,9 +828,6 @@ config CPU_BPREDICT_DISABLE
 	help
 	  Say Y here to disable branch prediction.  If unsure, say N.
 
-config CPU_SPECTRE
-	bool
-
 config HARDEN_BRANCH_PREDICTOR
 	bool "Harden the branch predictor against aliasing attacks" if EXPERT
 	depends on CPU_SPECTRE

arch/x86/Kconfig +1 -0

index d9830e7e1060..769739da67c6 100644
@@ -124,6 +124,7 @@ config X86
 	select CLKEVT_I8253
 	select CLOCKSOURCE_VALIDATE_LAST_CYCLE
 	select CLOCKSOURCE_WATCHDOG
+	select CPU_SPECTRE
 	select DCACHE_WORD_ACCESS
 	select EDAC_ATOMIC_SCRUB
 	select EDAC_SUPPORT

Signature · 1 line

2.31.1

Quoted 4 lines · Pawan Gupta

Borrow CONFIG_CPU_SPECTRE from ARM to be available for all architectures. This will help in configuration of features that depend on CPU being affected by spectre class of vulnerabilities.

Quoted 1 line

Signed-off-by Pawan Gupta <[email protected]>

Given that spectre isn't one specific issue, biut rather a blanket term for a bunch of things that can have variable overlap, I don't think this makes much sense unless we're going to add finer-grained options for all the variants, and IMO it'd make more sene for the architectures to directly select the things that'd otherwise be dependent on this.

Quoted 5 lines · Mark Rutland

Quoted 4 lines · Pawan Gupta

Borrow CONFIG_CPU_SPECTRE from ARM to be available for all architectures. This will help in configuration of features that depend on CPU being affected by spectre class of vulnerabilities.

Quoted 7 lines

Quoted 1 line

Signed-off-by Pawan Gupta <[email protected]>

Given that spectre isn't one specific issue, biut rather a blanket term for a bunch of things that can have variable overlap, I don't think this makes much sense unless we're going to add finer-grained options for all the variants, and IMO it'd make more sene for the architectures to directly select the things that'd otherwise be dependent on this.

Isn't ARM already using CPU_SPECTRE for selecting things:

config HARDEN_BRANCH_PREDICTOR
bool "Harden the branch predictor against aliasing attacks" if EXPERT depends on CPU_SPECTRE

This was the whole motivation for doing the same for x86.

Adding a condition for all architectures is also okay, but its going to a little messier:

	 config BPF_UNPRIV_DEFAULT_OFF
	        default y if X86 || ARM || ...

This approach would make sense if architectures wants to explicitly select the defaults irrespective of architecture being affected by spectre.

Quoted 14 lines · Pawan Gupta

Quoted 5 lines · Mark Rutland

Quoted 4 lines · Pawan Gupta

Borrow CONFIG_CPU_SPECTRE from ARM to be available for all architectures. This will help in configuration of features that depend on CPU being affected by spectre class of vulnerabilities.

Quoted 7 lines

Quoted 1 line

Signed-off-by Pawan Gupta <[email protected]>

Given that spectre isn't one specific issue, biut rather a blanket term for a bunch of things that can have variable overlap, I don't think this makes much sense unless we're going to add finer-grained options for all the variants, and IMO it'd make more sene for the architectures to directly select the things that'd otherwise be dependent on this.

Quoted 5 lines

Isn't ARM already using CPU_SPECTRE for selecting things:

config HARDEN_BRANCH_PREDICTOR
bool "Harden the branch predictor against aliasing attacks" if EXPERT depends on CPU_SPECTRE

It's true that arch/arm does, but that's not true for other architectures, e.g. powerpc or arm64, and and as above I don't think it makes sense to make this generic in its current form because "spectre" is a somewhat vague generic term.

Quoted 11 lines

This was the whole motivation for doing the same for x86.

Adding a condition for all architectures is also okay, but its going to a little messier:

	 config BPF_UNPRIV_DEFAULT_OFF
	        default y if X86 || ARM || ...

This approach would make sense if architectures wants to explicitly select the defaults irrespective of architecture being affected by spectre.

If we're going to change the default for some architectures, I think it'd make much more sense to just do that for all, without any arch-specific conditionality, i.e.

config BPF_UNPRIV_DEFAULT_OFF
default y

Quoted 38 lines · Mark Rutland

Quoted 14 lines · Pawan Gupta

Quoted 5 lines · Mark Rutland

Quoted 4 lines · Pawan Gupta

Borrow CONFIG_CPU_SPECTRE from ARM to be available for all architectures. This will help in configuration of features that depend on CPU being affected by spectre class of vulnerabilities.

Quoted 7 lines

Quoted 1 line

Signed-off-by Pawan Gupta <[email protected]>

Given that spectre isn't one specific issue, biut rather a blanket term for a bunch of things that can have variable overlap, I don't think this makes much sense unless we're going to add finer-grained options for all the variants, and IMO it'd make more sene for the architectures to directly select the things that'd otherwise be dependent on this.

Quoted 5 lines

Isn't ARM already using CPU_SPECTRE for selecting things:

config HARDEN_BRANCH_PREDICTOR
bool "Harden the branch predictor against aliasing attacks" if EXPERT depends on CPU_SPECTRE

It's true that arch/arm does, but that's not true for other architectures, e.g. powerpc or arm64, and and as above I don't think it makes sense to make this generic in its current form because "spectre" is a somewhat vague generic term.

Quoted 11 lines

This was the whole motivation for doing the same for x86.

Adding a condition for all architectures is also okay, but its going to a little messier:

	 config BPF_UNPRIV_DEFAULT_OFF
	        default y if X86 || ARM || ...

This approach would make sense if architectures wants to explicitly select the defaults irrespective of architecture being affected by spectre.

Quoted 6 lines

If we're going to change the default for some architectures, I think it'd make much more sense to just do that for all, without any arch-specific conditionality, i.e.

config BPF_UNPRIV_DEFAULT_OFF
default y

Lets just go with 'default y'. The main rationale for this change was motivated by spectre, so would have been good to indicate this also with an explicit dependency for broken HW, not just help description. Pretty much agreeing with Greg here [0]. Eventually, we might need some arch generic way to determine arch- common spectre type bugs, so that for unaffected HW we don't need to apply some of them from verifier, but that's still tbd.

Isn't ARM already using CPU_SPECTRE for selecting things:

config HARDEN_BRANCH_PREDICTOR
bool "Harden the branch predictor against aliasing attacks" if EXPERT depends on CPU_SPECTRE

This was the whole motivation for doing the same for x86.

Adding a condition for all architectures is also okay, but its going to a little messier:

	 config BPF_UNPRIV_DEFAULT_OFF
	        default y if X86 || ARM || ...

Quoted 6 lines · Pawan Gupta

Isn't ARM already using CPU_SPECTRE for selecting things:

config HARDEN_BRANCH_PREDICTOR
bool "Harden the branch predictor against aliasing attacks" if EXPERT depends on CPU_SPECTRE

Quoted 7 lines

This was the whole motivation for doing the same for x86.

Adding a condition for all architectures is also okay, but its going to a little messier:

	 config BPF_UNPRIV_DEFAULT_OFF
	        default y if X86 || ARM || ...

It doesn't have to be (but sadly we end up repeating "DEFAULT"):

config BPF_UNPRIV_DEFAULT_OFF_DEFAULT
bool

config BPF_UNPRIV_DEFAULT_OFF
bool "Disable unprivileged BPF by default" default BPF_UNPRIV_DEFAULT_OFF_DEFAULT

Then architectures can select BPF_UNPRIV_DEFAULT_OFF_DEFAULT if they wish this to be defaulted to "yes".

However, please note that this has limited use given that the BPF_UNPRIV_DEFAULT_OFF option has been around for a while now. Any existing configuration that mentions this symbol will override any default specified in the Kconfig files if the option is user-visible.

So, IMHO, defaults need to be set correctly from the point in time that the option is introduced.

Disabling unprivileged BPF would help prevent unprivileged users from creating the conditions required for potential speculative execution side-channel attacks on affected hardware. A deep dive on such attacks and mitigation is available here [1].

If an architecture selects CONFIG_CPU_SPECTRE, disable unprivileged BPF by default. An admin can enable this at runtime, if necessary.

Signed-off-by Pawan Gupta <[email protected]>

[1] https://ebpf.io/summit-2021-slides/eBPF_Summit_2021-Keynote-Daniel_Borkmann-BPF_and_Spectre.pdf

Quoted 8 lines · Pawan Gupta

Disabling unprivileged BPF would help prevent unprivileged users from creating the conditions required for potential speculative execution side-channel attacks on affected hardware. A deep dive on such attacks and mitigation is available here [1].

If an architecture selects CONFIG_CPU_SPECTRE, disable unprivileged BPF by default. An admin can enable this at runtime, if necessary.

Quoted 3 lines

Signed-off-by Pawan Gupta <[email protected]>

[1] https://ebpf.io/summit-2021-slides/eBPF_Summit_2021-Keynote-Daniel_Borkmann-BPF_and_Spectre.pdf

This should go above the signed-off-by line, in the changelog text, not below it, otherwise our tools get confused when trying to apply it.

Disabling unprivileged BPF would help prevent unprivileged users from creating the conditions required for potential speculative execution side-channel attacks on affected hardware. A deep dive on such attacks and mitigation is available here [1].

If an architecture selects CONFIG_CPU_SPECTRE, disable unprivileged BPF by default. An admin can enable this at runtime, if necessary.

Signed-off-by Pawan Gupta <[email protected]>

[1] https://ebpf.io/summit-2021-slides/eBPF_Summit_2021-Keynote-Daniel_Borkmann-BPF_and_Spectre.pdf

---
 kernel/bpf/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

kernel/bpf/Kconfig +0 -0

index a82d6de86522..510a5a73f9a2 100644

kernel/bpf/Kconfig +4 -0

 	depends on BPF_SYSCALL
 	help
 	  Disables unprivileged BPF by default by setting the corresponding

@@ -72,6 +73,10 @@ config BPF_UNPRIV_DEFAULT_OFF
 	  disable it by setting it to 1 (from which no other transition to
 	  0 is possible anymore).
 
+	  Unprivileged BPF can be used to exploit potential speculative
+	  execution side-channel vulnerabilities on affected hardware. If you
+	  are concerned about it, answer Y.
+
 source "kernel/bpf/preload/Kconfig"
 
 config BPF_LSM

Signature · 1 line

2.31.1

Quoted 21 lines · Pawan Gupta

Disabling unprivileged BPF would help prevent unprivileged users from creating the conditions required for potential speculative execution side-channel attacks on affected hardware. A deep dive on such attacks and mitigation is available here [1].

If an architecture selects CONFIG_CPU_SPECTRE, disable unprivileged BPF by default. An admin can enable this at runtime, if necessary.

Signed-off-by Pawan Gupta <[email protected]>

[1] https://ebpf.io/summit-2021-slides/eBPF_Summit_2021-Keynote-Daniel_Borkmann-BPF_and_Spectre.pdf

---
 kernel/bpf/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

kernel/bpf/Kconfig +0 -0

index a82d6de86522..510a5a73f9a2 100644
@@ -64,6 +64,7 @@ config BPF_JIT_DEFAULT_ON
 

kernel/bpf/Kconfig config BPF_JIT_DEFAULT_ON · line 67

Quoted 3 lines

kernel/bpf/Kconfig +1 -0

 config BPF_UNPRIV_DEFAULT_OFF
 	bool "Disable unprivileged BPF by default"
+	default y if CPU_SPECTRE

Why can't this just be "default y"?

This series makes that the case on x86, and if SW is going to have to deal with that we may as well do that everywhere, and say that on all architectures we leave it to the sysadmin or kernel builder to optin to permitting unprivileged BPF.

If we can change the default for x86 I see no reason we can't change this globally, and we avoid tying this to CPU_SPECTRE specifically.