ARM: multi_v7_defconfig: enable BPF related configs

The BPF components are getting more widely used by various components so we should enable them in the ARMv7 multi config to ensure they get wider testing and don't regress.

arch/arm64/configs/defconfig +1 -0

index c9a57d11330b..8748f6cf9fcd 100644
@@ -19,6 +19,7 @@ CONFIG_BLK_CGROUP=y
 CONFIG_CGROUP_PIDS=y
 CONFIG_CGROUP_HUGETLB=y
 CONFIG_CPUSETS=y
+CONFIG_CGROUP_BPF=y
 CONFIG_CGROUP_DEVICE=y
 CONFIG_CGROUP_CPUACCT=y
 CONFIG_CGROUP_PERF=y

arch/arm64/configs/defconfig +1 -0

+CONFIG_BPF_STREAM_PARSER=y
 CONFIG_BT=m
 CONFIG_BT_HIDP=m
 # CONFIG_BT_HS is not set

Signature · 1 line

2.19.1

Hi Peter, [+Jann and Ard]

Quoted 3 lines · Peter Robinson

The BPF components are getting more widely used by various components so we should enable them in the ARMv7 multi config to ensure they get wider testing and don't regress.

^^^ Looks like it needs updating for arm64?

Quoted 13 lines

arch/arm64/configs/defconfig +1 -0

index c9a57d11330b..8748f6cf9fcd 100644
@@ -19,6 +19,7 @@ CONFIG_BLK_CGROUP=y
 CONFIG_CGROUP_PIDS=y
 CONFIG_CGROUP_HUGETLB=y
 CONFIG_CPUSETS=y
+CONFIG_CGROUP_BPF=y
 CONFIG_CGROUP_DEVICE=y
 CONFIG_CGROUP_CPUACCT=y
 CONFIG_CGROUP_PERF=y

arch/arm64/configs/defconfig CONFIG_VLAN_8021Q_MVRP=y · line 163

Quoted 6 lines

arch/arm64/configs/defconfig +2 -0

@@ -157,7 +158,10 @@ CONFIG_VLAN_8021Q_MVRP=y
 CONFIG_QRTR=m
 CONFIG_QRTR_SMD=m
 CONFIG_QRTR_TUN=m
+CONFIG_BPF_SYSCALL=y
 CONFIG_BPF_JIT=y
+CONFIG_BPF_JIT_ALWAYS_ON=y

Have you considered the security impact of these two options at all? Whilst there appears to be a sysctl to disable unprivileged BPF usage, it's a one-way switch so it defaults to "allow unprivileged access" so we're opening up a pretty big attack surface by default with this change. My concerns are broadly:

1. BPF has a track record of serious security problems. This isn't intended as a criticism of the developers, it's more that the nature of the beast means it's a bit of a sitting duck. It's also constantly growing new functionality which would then likely be enabled by default under these options, so I fully expect things to continue as they are until the feature set has started to stabilise.

2. Whilst forcing the JIT to be "always on" removes the interpreter from the equation (and it's the interpreter which was heavily targetted by spectre-v2), it also has the nasty consequent of effectively allowing the modules area to be sprayed with executable code.

Now, this is only defconfig we're talking about here, but the impression I get from your patch is that your motivation is purely (ARMv7?) regression testing and I would prefer us at least to consider whether there are other implications from enabling this stuff by default on arm64.

Have other architectures already made this leap?

Quoted 7 lines · Will Deacon

Quoted 3 lines · Peter Robinson

The BPF components are getting more widely used by various components so we should enable them in the ARMv7 multi config to ensure they get wider testing and don't regress.

Have other architectures already made this leap?

$ git grep CONFIG_BPF_SYSCALL=y arch/
arch/arm/configs/aspeed_g4_defconfig:CONFIG_BPF_SYSCALL=y
arch/arm/configs/aspeed_g5_defconfig:CONFIG_BPF_SYSCALL=y
arch/mips/configs/generic_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/44x/fsp2_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/powernv_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/ppc64_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/pseries_defconfig:CONFIG_BPF_SYSCALL=y
arch/riscv/configs/defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/debug_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/performance_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/defconfig:CONFIG_BPF_SYSCALL=y

but nobody seems to enable CONFIG_BPF_JIT_ALWAYS_ON.

I sent some patches to move the BPF JIT allocations out of the module range. Whether that really improves things in terms of security is not obvious to me, but at least we stop wasting module region space (and potentially KASAN shadow pages) on BPF programs.

If this is mainly for coverage, it would indeed be nice if we could at least make it root only by default. However, if the distros are enabling this in their default configurations, I'd prefer it if we at least have a config that will help us spot issues early on.

Quoted 29 lines · Ard Biesheuvel

Quoted 7 lines · Will Deacon

Quoted 3 lines · Peter Robinson

The BPF components are getting more widely used by various components so we should enable them in the ARMv7 multi config to ensure they get wider testing and don't regress.

Have other architectures already made this leap?

$ git grep CONFIG_BPF_SYSCALL=y arch/
arch/arm/configs/aspeed_g4_defconfig:CONFIG_BPF_SYSCALL=y
arch/arm/configs/aspeed_g5_defconfig:CONFIG_BPF_SYSCALL=y
arch/mips/configs/generic_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/44x/fsp2_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/powernv_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/ppc64_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/pseries_defconfig:CONFIG_BPF_SYSCALL=y
arch/riscv/configs/defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/debug_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/performance_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/defconfig:CONFIG_BPF_SYSCALL=y

but nobody seems to enable CONFIG_BPF_JIT_ALWAYS_ON.

I sent some patches to move the BPF JIT allocations out of the module range. Whether that really improves things in terms of security is not obvious to me, but at least we stop wasting module region space (and potentially KASAN shadow pages) on BPF programs.

Quoted 4 lines

If this is mainly for coverage, it would indeed be nice if we could at least make it root only by default. However, if the distros are enabling this in their default configurations, I'd prefer it if we at least have a config that will help us spot issues early on.

That's a fair point on the distros. Peter, as author of the patch, please can you take a look at the arm64 kernel configs from some popular distributions and see which of these options they tend to enable?

Quoted 30 lines · Will Deacon

Quoted 29 lines · Ard Biesheuvel

Quoted 7 lines · Will Deacon

Quoted 3 lines · Peter Robinson

The BPF components are getting more widely used by various components so we should enable them in the ARMv7 multi config to ensure they get wider testing and don't regress.

Have other architectures already made this leap?

$ git grep CONFIG_BPF_SYSCALL=y arch/
arch/arm/configs/aspeed_g4_defconfig:CONFIG_BPF_SYSCALL=y
arch/arm/configs/aspeed_g5_defconfig:CONFIG_BPF_SYSCALL=y
arch/mips/configs/generic_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/44x/fsp2_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/powernv_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/ppc64_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/pseries_defconfig:CONFIG_BPF_SYSCALL=y
arch/riscv/configs/defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/debug_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/performance_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/defconfig:CONFIG_BPF_SYSCALL=y

but nobody seems to enable CONFIG_BPF_JIT_ALWAYS_ON.

I sent some patches to move the BPF JIT allocations out of the module range. Whether that really improves things in terms of security is not obvious to me, but at least we stop wasting module region space (and potentially KASAN shadow pages) on BPF programs.

Quoted 8 lines

Quoted 4 lines

If this is mainly for coverage, it would indeed be nice if we could at least make it root only by default. However, if the distros are enabling this in their default configurations, I'd prefer it if we at least have a config that will help us spot issues early on.

That's a fair point on the distros. Peter, as author of the patch, please can you take a look at the arm64 kernel configs from some popular distributions and see which of these options they tend to enable?

Another point to consider is whether the BPF JIT creates code that is safe from Spectre, or whether it opens up more Spectre issues. If it isn't, maybe the better option is to disable kernel-side BPF entirely until it is fixed up.

I've raised that point for 32-bit ARM already, and the 32-bit ARM BPF JIT isn't going to have the Spectre mitigations applied (since no one appears interested whenever I've raised it.) So, for 32-bit ARM, it's better to have kernel-side BPF entirely disabled if you care about Spectre mitigations.

Hi Russell,

Quoted 45 lines · Russell King - ARM Linux

Quoted 30 lines · Will Deacon

Quoted 29 lines · Ard Biesheuvel

Quoted 7 lines · Will Deacon

Quoted 3 lines · Peter Robinson

The BPF components are getting more widely used by various components so we should enable them in the ARMv7 multi config to ensure they get wider testing and don't regress.

Have other architectures already made this leap?

$ git grep CONFIG_BPF_SYSCALL=y arch/
arch/arm/configs/aspeed_g4_defconfig:CONFIG_BPF_SYSCALL=y
arch/arm/configs/aspeed_g5_defconfig:CONFIG_BPF_SYSCALL=y
arch/mips/configs/generic_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/44x/fsp2_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/powernv_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/ppc64_defconfig:CONFIG_BPF_SYSCALL=y
arch/powerpc/configs/pseries_defconfig:CONFIG_BPF_SYSCALL=y
arch/riscv/configs/defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/debug_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/configs/performance_defconfig:CONFIG_BPF_SYSCALL=y
arch/s390/defconfig:CONFIG_BPF_SYSCALL=y

but nobody seems to enable CONFIG_BPF_JIT_ALWAYS_ON.

I sent some patches to move the BPF JIT allocations out of the module range. Whether that really improves things in terms of security is not obvious to me, but at least we stop wasting module region space (and potentially KASAN shadow pages) on BPF programs.

Quoted 8 lines

Quoted 4 lines

If this is mainly for coverage, it would indeed be nice if we could at least make it root only by default. However, if the distros are enabling this in their default configurations, I'd prefer it if we at least have a config that will help us spot issues early on.

That's a fair point on the distros. Peter, as author of the patch, please can you take a look at the arm64 kernel configs from some popular distributions and see which of these options they tend to enable?

Another point to consider is whether the BPF JIT creates code that is safe from Spectre, or whether it opens up more Spectre issues. If it isn't, maybe the better option is to disable kernel-side BPF entirely until it is fixed up.

Quoted 5 lines

I've raised that point for 32-bit ARM already, and the 32-bit ARM BPF JIT isn't going to have the Spectre mitigations applied (since no one appears interested whenever I've raised it.) So, for 32-bit ARM, it's better to have kernel-side BPF entirely disabled if you care about Spectre mitigations.

I thought that the Spectre mitigations (at least for variant 1) were handled by the core BPF JIT, as a result of b2157399cc98 ("bpf: prevent out-of-bounds speculation")? I think we rely on randomization to protect against variant 2 for JITted code.

Did I miss something here?

Quoted 12 lines · Will Deacon

Quoted 10 lines · Russell King - ARM Linux

Another point to consider is whether the BPF JIT creates code that is safe from Spectre, or whether it opens up more Spectre issues. If it isn't, maybe the better option is to disable kernel-side BPF entirely until it is fixed up.

I've raised that point for 32-bit ARM already, and the 32-bit ARM BPF JIT isn't going to have the Spectre mitigations applied (since no one appears interested whenever I've raised it.) So, for 32-bit ARM, it's better to have kernel-side BPF entirely disabled if you care about Spectre mitigations.

Quoted 6 lines

I thought that the Spectre mitigations (at least for variant 1) were handled by the core BPF JIT, as a result of b2157399cc98 ("bpf: prevent out-of-bounds speculation")? I think we rely on randomization to protect against variant 2 for JITted code.

Did I miss something here?

It depends - there's at least one case in eBPF that requires the JIT to be mitigated, and that's the tail-call stuff. That's prime Spectre exploitation territory, because the pseudocode for that eBPF instruction is:

	if (index >= array->map.max_entries)
		goto out;
	if (tail_call_cnt > MAX_TAIL_CALL_CNT)
		goto out;
	tail_call_cnt++;
	prog = array->ptrs[index];
	if (prog == NULL)
		goto out;
	goto *(prog->bpf_func + prologue_size);

If 'index' is larger than map.max_entries, and we speculate past the goto, then out of bounds cache lines in the array->ptrs[] could be loaded.

In cBPF, it seems to me that there is enough support to do something like:

	if (val > max)		// BPF_JMP | BPF_JGT | BPF_K
		jump somewhere;
	addr += val;		// BPF_ALU | BPF_ADD | BPF_W
	val2 = *addr;		// BPF_LDX | BPF_MEM | BPF_W

which looks to me like it can fit the Spectre exploitation pattern, but is much harder to work around than the eBPF tailcall (we would need to detect the BPF instruction sequence.) I haven't validated that such a sequence is actually possible in cBPF, but as cBPF is used for packet filtering, and there's a need for it to support jumping over the headers which can be variable lengths, then I would not be surprised if such BPF sequences were possible.