All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH ebpf v2 0/2] Unprivileged BPF default
@ 2021-10-28  1:32 Pawan Gupta
  2021-10-28  1:33   ` Pawan Gupta
  2021-10-28  1:35 ` [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE Pawan Gupta
  0 siblings, 2 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-28  1:32 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, netdev, bpf, linux-kernel,
	antonio.gomez.iglesias, tony.luck, dave.hansen

Hi,

This is a two patch series to make the compile time default of
unprivileged BPF depend on CONFIG_CPU_SPECTRE. First patch makes ARM's
CONFIG_CPU_SPECTRE available for all architectures. The second patch
sets CONFIG_BPF_UNPRIV_DEFAULT_OFF=y by default when
CONFIG_CPU_SPECTRE=y.

v2:
- Generalize ARM's CONFIG_CPU_SPECTRE to be available for all architectures.
- Make CONFIG_BPF_UNPRIV_DEFAULT_OFF depend on CONFIG_CPU_SPECTRE.
- Updated commit message to reflect the dependency on CONFIG_CPU_SPECTRE.
- Add reference to BPF spectre presentation in commit message.

v1: https://lore.kernel.org/all/d37b01e70e65dced2659561ed5bc4b2ed1a50711.1635367330.git.pawan.kumar.gupta@linux.intel.com/

Pawan Gupta (2):
  arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE

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

-- 
2.31.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-28  1:32 [PATCH ebpf v2 0/2] Unprivileged BPF default Pawan Gupta
@ 2021-10-28  1:33   ` Pawan Gupta
  2021-10-28  1:35 ` [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE Pawan Gupta
  1 sibling, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-28  1:33 UTC (permalink / raw)
  To: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
  Cc: x86, H. Peter Anvin, Kees Cook, Andrew Morton, Masahiro Yamada,
	Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Mark Rutland, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Pawan Gupta,
	Nick Desaulniers, linux-kernel, linux-arm-kernel

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 <pawan.kumar.gupta@linux.intel.com>
---
 arch/Kconfig        | 3 +++
 arch/arm/mm/Kconfig | 3 ---
 arch/x86/Kconfig    | 1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8df1c7102643..6aa856d51cb7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -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
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 8355c3895894..44551465fd03 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -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
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d9830e7e1060..769739da67c6 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-28  1:33   ` Pawan Gupta
  0 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-28  1:33 UTC (permalink / raw)
  To: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
  Cc: x86, H. Peter Anvin, Kees Cook, Andrew Morton, Masahiro Yamada,
	Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Mark Rutland, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Pawan Gupta,
	Nick Desaulniers, linux-kernel, linux-arm-kernel

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 <pawan.kumar.gupta@linux.intel.com>
---
 arch/Kconfig        | 3 +++
 arch/arm/mm/Kconfig | 3 ---
 arch/x86/Kconfig    | 1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8df1c7102643..6aa856d51cb7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -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
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 8355c3895894..44551465fd03 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -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
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d9830e7e1060..769739da67c6 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE
  2021-10-28  1:32 [PATCH ebpf v2 0/2] Unprivileged BPF default Pawan Gupta
  2021-10-28  1:33   ` Pawan Gupta
@ 2021-10-28  1:35 ` Pawan Gupta
  2021-10-28  5:34   ` Greg KH
  2021-10-28 13:57   ` Mark Rutland
  1 sibling, 2 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-28  1:35 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, netdev, bpf, linux-kernel,
	antonio.gomez.iglesias, tony.luck, dave.hansen, gregkh

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 <pawan.kumar.gupta@linux.intel.com>

[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(+)

diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
index a82d6de86522..510a5a73f9a2 100644
--- a/kernel/bpf/Kconfig
+++ b/kernel/bpf/Kconfig
@@ -64,6 +64,7 @@ config BPF_JIT_DEFAULT_ON
 
 config BPF_UNPRIV_DEFAULT_OFF
 	bool "Disable unprivileged BPF by default"
+	default y if CPU_SPECTRE
 	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
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-28  1:33   ` Pawan Gupta
@ 2021-10-28  5:27     ` Greg KH
  -1 siblings, 0 replies; 24+ messages in thread
From: Greg KH @ 2021-10-28  5:27 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Mark Rutland, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel

On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
> 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 <pawan.kumar.gupta@linux.intel.com>
> ---
>  arch/Kconfig        | 3 +++
>  arch/arm/mm/Kconfig | 3 ---
>  arch/x86/Kconfig    | 1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)

You forgot to cc: the bpf list for this.

greg k-h

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-28  5:27     ` Greg KH
  0 siblings, 0 replies; 24+ messages in thread
From: Greg KH @ 2021-10-28  5:27 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Mark Rutland, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel

On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
> 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 <pawan.kumar.gupta@linux.intel.com>
> ---
>  arch/Kconfig        | 3 +++
>  arch/arm/mm/Kconfig | 3 ---
>  arch/x86/Kconfig    | 1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)

You forgot to cc: the bpf list for this.

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE
  2021-10-28  1:35 ` [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE Pawan Gupta
@ 2021-10-28  5:34   ` Greg KH
  2021-10-28 19:38     ` Pawan Gupta
  2021-10-28 13:57   ` Mark Rutland
  1 sibling, 1 reply; 24+ messages in thread
From: Greg KH @ 2021-10-28  5:34 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, netdev, bpf, linux-kernel, antonio.gomez.iglesias,
	tony.luck, dave.hansen

On Wed, Oct 27, 2021 at 06:35:44PM -0700, Pawan Gupta wrote:
> 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 <pawan.kumar.gupta@linux.intel.com>
> 
> [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.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-28  1:33   ` Pawan Gupta
@ 2021-10-28 13:49     ` Mark Rutland
  -1 siblings, 0 replies; 24+ messages in thread
From: Mark Rutland @ 2021-10-28 13:49 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel

On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
> 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 <pawan.kumar.gupta@linux.intel.com>

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.

Thanks,
Mark.

> ---
>  arch/Kconfig        | 3 +++
>  arch/arm/mm/Kconfig | 3 ---
>  arch/x86/Kconfig    | 1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8df1c7102643..6aa856d51cb7 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -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
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index 8355c3895894..44551465fd03 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -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
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d9830e7e1060..769739da67c6 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -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
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-28 13:49     ` Mark Rutland
  0 siblings, 0 replies; 24+ messages in thread
From: Mark Rutland @ 2021-10-28 13:49 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel

On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
> 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 <pawan.kumar.gupta@linux.intel.com>

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.

Thanks,
Mark.

> ---
>  arch/Kconfig        | 3 +++
>  arch/arm/mm/Kconfig | 3 ---
>  arch/x86/Kconfig    | 1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8df1c7102643..6aa856d51cb7 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -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
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index 8355c3895894..44551465fd03 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -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
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d9830e7e1060..769739da67c6 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -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
> -- 
> 2.31.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE
  2021-10-28  1:35 ` [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE Pawan Gupta
  2021-10-28  5:34   ` Greg KH
@ 2021-10-28 13:57   ` Mark Rutland
  2021-10-28 16:44     ` Greg KH
  1 sibling, 1 reply; 24+ messages in thread
From: Mark Rutland @ 2021-10-28 13:57 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, netdev, bpf, linux-kernel, antonio.gomez.iglesias,
	tony.luck, dave.hansen, gregkh

On Wed, Oct 27, 2021 at 06:35:44PM -0700, Pawan Gupta wrote:
> 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 <pawan.kumar.gupta@linux.intel.com>
> 
> [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(+)
> 
> diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
> index a82d6de86522..510a5a73f9a2 100644
> --- a/kernel/bpf/Kconfig
> +++ b/kernel/bpf/Kconfig
> @@ -64,6 +64,7 @@ config BPF_JIT_DEFAULT_ON
>  
>  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.

Thanks,
Mark.

>  	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
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE
  2021-10-28 13:57   ` Mark Rutland
@ 2021-10-28 16:44     ` Greg KH
  0 siblings, 0 replies; 24+ messages in thread
From: Greg KH @ 2021-10-28 16:44 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Pawan Gupta, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, netdev, bpf, linux-kernel,
	antonio.gomez.iglesias, tony.luck, dave.hansen

On Thu, Oct 28, 2021 at 02:57:51PM +0100, Mark Rutland wrote:
> On Wed, Oct 27, 2021 at 06:35:44PM -0700, Pawan Gupta wrote:
> > 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 <pawan.kumar.gupta@linux.intel.com>
> > 
> > [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(+)
> > 
> > diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
> > index a82d6de86522..510a5a73f9a2 100644
> > --- a/kernel/bpf/Kconfig
> > +++ b/kernel/bpf/Kconfig
> > @@ -64,6 +64,7 @@ config BPF_JIT_DEFAULT_ON
> >  
> >  config BPF_UNPRIV_DEFAULT_OFF
> >  	bool "Disable unprivileged BPF by default"
> > +	default y if CPU_SPECTRE
> 
> Why can't this just be "default y"?

Because not all arches are broken.

> 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.

No, this is a spectre-like issue only, if you have hardware that does
not have these types of issues, why wouldn't this be ok to be disabled?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-28 13:49     ` Mark Rutland
@ 2021-10-28 19:36       ` Pawan Gupta
  -1 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-28 19:36 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On 28.10.2021 14:49, Mark Rutland wrote:
>On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
>> 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 <pawan.kumar.gupta@linux.intel.com>
>
>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.

If that's the case I will change the BPF_UNPRIV_DEFAULT_OFF default to
depend on architecture. I hope BPF maintainer, Daniel is okay with it?

Pawan

Added BPF maintainers and bpf@vger to cc.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-28 19:36       ` Pawan Gupta
  0 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-28 19:36 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On 28.10.2021 14:49, Mark Rutland wrote:
>On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
>> 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 <pawan.kumar.gupta@linux.intel.com>
>
>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.

If that's the case I will change the BPF_UNPRIV_DEFAULT_OFF default to
depend on architecture. I hope BPF maintainer, Daniel is okay with it?

Pawan

Added BPF maintainers and bpf@vger to cc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE
  2021-10-28  5:34   ` Greg KH
@ 2021-10-28 19:38     ` Pawan Gupta
  0 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-28 19:38 UTC (permalink / raw)
  To: Greg KH
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, netdev, bpf, linux-kernel, antonio.gomez.iglesias,
	tony.luck, dave.hansen

On 28.10.2021 07:34, Greg KH wrote:
>On Wed, Oct 27, 2021 at 06:35:44PM -0700, Pawan Gupta wrote:
>> 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 <pawan.kumar.gupta@linux.intel.com>
>>
>> [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.

Thanks, I will fix it.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-28 19:36       ` Pawan Gupta
@ 2021-10-29  9:22         ` Mark Rutland
  -1 siblings, 0 replies; 24+ messages in thread
From: Mark Rutland @ 2021-10-29  9:22 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
> On 28.10.2021 14:49, Mark Rutland wrote:
> > On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
> > > 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 <pawan.kumar.gupta@linux.intel.com>
> > 
> > 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

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.

> 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

... so that the behaviour is consistent across all architectures, and we
don't have to play a whack-a-mole game as/when we realise architectures
are affected by some variant of an issue relating to speculation.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-29  9:22         ` Mark Rutland
  0 siblings, 0 replies; 24+ messages in thread
From: Mark Rutland @ 2021-10-29  9:22 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
> On 28.10.2021 14:49, Mark Rutland wrote:
> > On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
> > > 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 <pawan.kumar.gupta@linux.intel.com>
> > 
> > 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

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.

> 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

... so that the behaviour is consistent across all architectures, and we
don't have to play a whack-a-mole game as/when we realise architectures
are affected by some variant of an issue relating to speculation.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-28 19:36       ` Pawan Gupta
@ 2021-10-29 10:08         ` Russell King (Oracle)
  -1 siblings, 0 replies; 24+ messages in thread
From: Russell King (Oracle) @ 2021-10-29 10:08 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
> 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 || ...

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-29 10:08         ` Russell King (Oracle)
  0 siblings, 0 replies; 24+ messages in thread
From: Russell King (Oracle) @ 2021-10-29 10:08 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
> 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 || ...

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-29  9:22         ` Mark Rutland
@ 2021-10-29 16:05           ` Daniel Borkmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Daniel Borkmann @ 2021-10-29 16:05 UTC (permalink / raw)
  To: Mark Rutland, Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov, bpf

On 10/29/21 11:22 AM, Mark Rutland wrote:
> On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
>> On 28.10.2021 14:49, Mark Rutland wrote:
>>> On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
>>>> 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 <pawan.kumar.gupta@linux.intel.com>
>>>
>>> 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
> 
> 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.
> 
>> 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

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.

Thanks,
Daniel

   [0] https://lore.kernel.org/bpf/YXrTev6WMXry9pFI@kroah.com/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-29 16:05           ` Daniel Borkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Borkmann @ 2021-10-29 16:05 UTC (permalink / raw)
  To: Mark Rutland, Pawan Gupta
  Cc: Russell King, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov, bpf

On 10/29/21 11:22 AM, Mark Rutland wrote:
> On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
>> On 28.10.2021 14:49, Mark Rutland wrote:
>>> On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
>>>> 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 <pawan.kumar.gupta@linux.intel.com>
>>>
>>> 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
> 
> 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.
> 
>> 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

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.

Thanks,
Daniel

   [0] https://lore.kernel.org/bpf/YXrTev6WMXry9pFI@kroah.com/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-29 16:05           ` Daniel Borkmann
@ 2021-10-29 19:15             ` Pawan Gupta
  -1 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-29 19:15 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Mark Rutland, Russell King, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Kees Cook,
	Andrew Morton, Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov, bpf

On 29.10.2021 18:05, Daniel Borkmann wrote:
>On 10/29/21 11:22 AM, Mark Rutland wrote:
>>On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
>>>On 28.10.2021 14:49, Mark Rutland wrote:
>>>>On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
>>>>>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 <pawan.kumar.gupta@linux.intel.com>
>>>>
>>>>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
>>
>>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.
>>
>>>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
>
>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.

I will send a patch soon with 'default y'.

Thanks,
Pawan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-29 19:15             ` Pawan Gupta
  0 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-29 19:15 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Mark Rutland, Russell King, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Kees Cook,
	Andrew Morton, Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov, bpf

On 29.10.2021 18:05, Daniel Borkmann wrote:
>On 10/29/21 11:22 AM, Mark Rutland wrote:
>>On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
>>>On 28.10.2021 14:49, Mark Rutland wrote:
>>>>On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote:
>>>>>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 <pawan.kumar.gupta@linux.intel.com>
>>>>
>>>>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
>>
>>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.
>>
>>>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
>
>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.

I will send a patch soon with 'default y'.

Thanks,
Pawan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
  2021-10-29 10:08         ` Russell King (Oracle)
@ 2021-10-29 22:51           ` Pawan Gupta
  -1 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-29 22:51 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On 29.10.2021 11:08, Russell King (Oracle) wrote:
>On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
>> 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 || ...
>
>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".

Looks like we are settling on unconditional 'default y' for now [1].
I have sent a v3 with 'default y' [2].

>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.

Yes, existing configurations will have to toggle this manually. However,
many distros already have BPF_UNPRIV_DEFAULT_OFF=y in their
configuration.

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

Agree.

[1] https://lore.kernel.org/lkml/6130e55f-4d84-5ada-4e86-5b678e3eaf5e@iogearbox.net/
[2] https://lore.kernel.org/lkml/0ace9ce3f97656d5f62d11093ad7ee81190c3c25.1635535215.git.pawan.kumar.gupta@linux.intel.com/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures
@ 2021-10-29 22:51           ` Pawan Gupta
  0 siblings, 0 replies; 24+ messages in thread
From: Pawan Gupta @ 2021-10-29 22:51 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Kees Cook, Andrew Morton,
	Masahiro Yamada, Peter Zijlstra (Intel),
	Sami Tolvanen, Colin Ian King, Frederic Weisbecker,
	Mike Rapoport, YiFei Zhu, Steven Rostedt (VMware),
	Viresh Kumar, Andrey Konovalov, Wang Kefeng,
	Uwe Kleine-König, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-arm-kernel, Alexei Starovoitov,
	Daniel Borkmann, bpf

On 29.10.2021 11:08, Russell King (Oracle) wrote:
>On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote:
>> 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 || ...
>
>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".

Looks like we are settling on unconditional 'default y' for now [1].
I have sent a v3 with 'default y' [2].

>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.

Yes, existing configurations will have to toggle this manually. However,
many distros already have BPF_UNPRIV_DEFAULT_OFF=y in their
configuration.

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

Agree.

[1] https://lore.kernel.org/lkml/6130e55f-4d84-5ada-4e86-5b678e3eaf5e@iogearbox.net/
[2] https://lore.kernel.org/lkml/0ace9ce3f97656d5f62d11093ad7ee81190c3c25.1635535215.git.pawan.kumar.gupta@linux.intel.com/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2021-10-29 22:50 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-28  1:32 [PATCH ebpf v2 0/2] Unprivileged BPF default Pawan Gupta
2021-10-28  1:33 ` [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures Pawan Gupta
2021-10-28  1:33   ` Pawan Gupta
2021-10-28  5:27   ` Greg KH
2021-10-28  5:27     ` Greg KH
2021-10-28 13:49   ` Mark Rutland
2021-10-28 13:49     ` Mark Rutland
2021-10-28 19:36     ` Pawan Gupta
2021-10-28 19:36       ` Pawan Gupta
2021-10-29  9:22       ` Mark Rutland
2021-10-29  9:22         ` Mark Rutland
2021-10-29 16:05         ` Daniel Borkmann
2021-10-29 16:05           ` Daniel Borkmann
2021-10-29 19:15           ` Pawan Gupta
2021-10-29 19:15             ` Pawan Gupta
2021-10-29 10:08       ` Russell King (Oracle)
2021-10-29 10:08         ` Russell King (Oracle)
2021-10-29 22:51         ` Pawan Gupta
2021-10-29 22:51           ` Pawan Gupta
2021-10-28  1:35 ` [PATCH ebpf v2 2/2] bpf: Make unprivileged bpf depend on CONFIG_CPU_SPECTRE Pawan Gupta
2021-10-28  5:34   ` Greg KH
2021-10-28 19:38     ` Pawan Gupta
2021-10-28 13:57   ` Mark Rutland
2021-10-28 16:44     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.