public inbox for linux-kselftest@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL
@ 2026-01-27 11:59 Luis Gerhorst
  2026-01-27 11:59 ` [PATCH bpf-next 1/2] " Luis Gerhorst
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Luis Gerhorst @ 2026-01-27 11:59 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Luis Gerhorst, Ihor Solodrai, Kumar Kartikeya Dwivedi,
	bpf, linux-kernel, linux-kselftest

This fixes the verifier_bug_if() that runs on nospec_result to not trigger
for BPF_CALL (bug reported by Hu, Mei, and Mu). See patch 1 for a full
description and patch 2 for a test (based on the PoC from the report).

While working on this I noticed two other problems:

- nospec_result is currently ignored for BPF_CALL during patching, but it
  may be required if we assume the CPU may speculate into/out of functions.

- Both the instruction patching for nospec and nospec_result erases the
  instruction aux information even thought it might be better to keep that.
  For nospec_result it may be fine as it is only applied to store
  instructions currently (except for when we decide to change the thing
  from above), but nospec may be set for arbitrary instructions and if
  these require rewrites they break.

I assume these issues are better fixed separately, thus I decided to
exclude them from this series.

Luis Gerhorst (2):
  bpf: Fix verifier_bug_if to account for BPF_CALL
  bpf: Test nospec after dead stack write in helper

 kernel/bpf/verifier.c                         | 14 +++++++-----
 .../selftests/bpf/progs/verifier_unpriv.c     | 22 +++++++++++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)


base-commit: 8016abd6314ed1ed01ff09404e3c82ceb13c185b
-- 
2.52.0


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

* [PATCH bpf-next 1/2] bpf: Fix verifier_bug_if to account for BPF_CALL
  2026-01-27 11:59 [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
@ 2026-01-27 11:59 ` Luis Gerhorst
  2026-01-27 11:59 ` [PATCH bpf-next 2/2] bpf: Test nospec after dead stack write in helper Luis Gerhorst
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Luis Gerhorst @ 2026-01-27 11:59 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Luis Gerhorst, Ihor Solodrai, Kumar Kartikeya Dwivedi,
	bpf, linux-kernel, linux-kselftest
  Cc: Yinhao Hu, Kaiyan Mei, Dongliang Mu

The BPF verifier assumes `insn_aux->nospec_result` is only set for
direct memory writes (e.g., `*(u32*)(r1+off) = r2`). However, the
assertion fails to account for helper calls (e.g.,
`bpf_skb_load_bytes_relative`) that perform writes to stack memory. Make
the check more precise to resolve this.

The problem is that `BPF_CALL` instructions have `BPF_CLASS(insn->code)
== BPF_JMP`, which triggers the warning check:

- Helpers like `bpf_skb_load_bytes_relative` write to stack memory
- `check_helper_call()` loops through `meta.access_size`, calling
  `check_mem_access(..., BPF_WRITE)`
- `check_stack_write()` sets `insn_aux->nospec_result = 1`
- Since `BPF_CALL` is encoded as `BPF_JMP | BPF_CALL`, the warning fires

Execution flow:

```
1. Drop capabilities → Enable Spectre mitigation
2. Load BPF program
   └─> do_check()
       ├─> check_cond_jmp_op() → Marks dead branch as speculative
       │   └─> push_stack(..., speculative=true)
       ├─> pop_stack() → state->speculative = 1
       ├─> check_helper_call() → Processes helper in dead branch
       │   └─> check_mem_access(..., BPF_WRITE)
       │       └─> insn_aux->nospec_result = 1
       └─> Checks: state->speculative && insn_aux->nospec_result
           └─> BPF_CLASS(insn->code) == BPF_JMP → WARNING
```

To fix the assert, it would be nice to be able to reuse
bpf_insn_successors() here, but bpf_insn_successors()->cnt is not
exactly what we want as it may also be 1 for BPF_JA. Instead, we could
check opcode_info.can_jump, but then we would have to share the table
between the functions. This would mean moving the table out of the
function and adding bpf_opcode_info(). As the verifier_bug_if() only
runs for insns with nospec_result set, the impact on verification time
would likely still be negligible. However, I assume sharing
bpf_opcode_info() between liveness.c and verifier.c will not be worth
it. It seems as only adjust_jmp_off() could also be simplified using it,
and there imm/off is touched. Thus it is maybe better to rely on exact
opcode/class matching there.

Therefore, to avoid this sharing only for a verifier_bug_if(), just
check the opcode. This should now cover all opcodes for which can_jump
in bpf_insn_successors() is true.

Parts of the description and example are taken from the bug report.

Fixes: dadb59104c64 ("bpf: Fix aux usage after do_check_insn()")
Signed-off-by: Luis Gerhorst <luis.gerhorst@fau.de>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
Closes: https://lore.kernel.org/bpf/7678017d-b760-4053-a2d8-a6879b0dbeeb@hust.edu.cn/
---
 kernel/bpf/verifier.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c2f2650db9fd..e7ff8394e0da 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21065,17 +21065,19 @@ static int do_check(struct bpf_verifier_env *env)
 			 * may skip a nospec patched-in after the jump. This can
 			 * currently never happen because nospec_result is only
 			 * used for the write-ops
-			 * `*(size*)(dst_reg+off)=src_reg|imm32` which must
-			 * never skip the following insn. Still, add a warning
-			 * to document this in case nospec_result is used
-			 * elsewhere in the future.
+			 * `*(size*)(dst_reg+off)=src_reg|imm32` and helper
+			 * calls. These must never skip the following insn
+			 * (i.e., bpf_insn_successors()'s opcode_info.can_jump
+			 * is false). Still, add a warning to document this in
+			 * case nospec_result is used elsewhere in the future.
 			 *
 			 * All non-branch instructions have a single
 			 * fall-through edge. For these, nospec_result should
 			 * already work.
 			 */
-			if (verifier_bug_if(BPF_CLASS(insn->code) == BPF_JMP ||
-					    BPF_CLASS(insn->code) == BPF_JMP32, env,
+			if (verifier_bug_if((BPF_CLASS(insn->code) == BPF_JMP ||
+					     BPF_CLASS(insn->code) == BPF_JMP32) &&
+					    BPF_OP(insn->code) != BPF_CALL, env,
 					    "speculation barrier after jump instruction may not have the desired effect"))
 				return -EFAULT;
 process_bpf_exit:
-- 
2.52.0


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

* [PATCH bpf-next 2/2] bpf: Test nospec after dead stack write in helper
  2026-01-27 11:59 [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
  2026-01-27 11:59 ` [PATCH bpf-next 1/2] " Luis Gerhorst
@ 2026-01-27 11:59 ` Luis Gerhorst
  2026-01-27 12:37 ` [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
  2026-01-29  2:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Luis Gerhorst @ 2026-01-27 11:59 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Luis Gerhorst, Ihor Solodrai, Kumar Kartikeya Dwivedi,
	bpf, linux-kernel, linux-kselftest
  Cc: Yinhao Hu, Kaiyan Mei, Dongliang Mu

Without the fix from the previous commit, the selftest fails:

$ ./tools/testing/selftests/bpf/vmtest.sh -- \
        ./test_progs -t verifier_unpriv
[...]
run_subtest:PASS:obj_open_mem 0 nsec
libbpf: BTF loading error: -EPERM
libbpf: Error loading .BTF into kernel: -EPERM. BTF is optional, ignoring.
libbpf: prog 'unpriv_nospec_after_helper_stack_write': BPF program load failed: -EFAULT
libbpf: prog 'unpriv_nospec_after_helper_stack_write': failed to load: -EFAULT
libbpf: failed to load object 'verifier_unpriv'
run_subtest:FAIL:unexpected_load_failure unexpected error: -14 (errno 14)
VERIFIER LOG:
=============
0: R1=ctx() R10=fp0
0: (b7) r0 = 0                        ; R0=P0
1: (55) if r0 != 0x1 goto pc+6 2: R0=Pscalar() R1=ctx() R10=fp0
2: (b7) r2 = 0                        ; R2=P0
3: (bf) r3 = r10                      ; R3=fp0 R10=fp0
4: (07) r3 += -16                     ; R3=fp-16
5: (b7) r4 = 4                        ; R4=P4
6: (b7) r5 = 0                        ; R5=P0
7: (85) call bpf_skb_load_bytes_relative#68
verifier bug: speculation barrier after jump instruction may not have the desired effect (BPF_CLASS(insn->code) == BPF_JMP || BPF_CLASS(insn->code) == BPF_JMP32)
processed 9 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
=============
[...]

The test is based on the PoC from the report.

Signed-off-by: Luis Gerhorst <luis.gerhorst@fau.de>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
Link: https://lore.kernel.org/bpf/7678017d-b760-4053-a2d8-a6879b0dbeeb@hust.edu.cn/
---
 .../selftests/bpf/progs/verifier_unpriv.c     | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_unpriv.c b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
index 28b4f7035ceb..8ee1243e62a8 100644
--- a/tools/testing/selftests/bpf/progs/verifier_unpriv.c
+++ b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
@@ -950,4 +950,26 @@ l3_%=:	r0 = 0;						\
 "	::: __clobber_all);
 }
 
+SEC("socket")
+__description("unpriv: nospec after dead stack write in helper")
+__success __success_unpriv
+__retval(0)
+/* Dead code sanitizer rewrites the call to `goto -1`. */
+__naked void unpriv_dead_helper_stack_write_nospec_result(void)
+{
+	asm volatile ("					\
+	r0 = 0;						\
+	if r0 != 1 goto l0_%=;				\
+	r2 = 0;						\
+	r3 = r10;					\
+	r3 += -16;					\
+	r4 = 4;						\
+	r5 = 0;						\
+	call %[bpf_skb_load_bytes_relative];		\
+l0_%=:	exit;						\
+"	:
+	: __imm(bpf_skb_load_bytes_relative)
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.52.0


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

* Re: [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL
  2026-01-27 11:59 [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
  2026-01-27 11:59 ` [PATCH bpf-next 1/2] " Luis Gerhorst
  2026-01-27 11:59 ` [PATCH bpf-next 2/2] bpf: Test nospec after dead stack write in helper Luis Gerhorst
@ 2026-01-27 12:37 ` Luis Gerhorst
  2026-01-29  2:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Luis Gerhorst @ 2026-01-27 12:37 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, John Fastabend, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	Ihor Solodrai, Kumar Kartikeya Dwivedi, bpf, linux-kernel,
	linux-kselftest

This series be applied to the bpf tree of course (not bpf-next), sorry
for the oversight.

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

* Re: [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL
  2026-01-27 11:59 [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
                   ` (2 preceding siblings ...)
  2026-01-27 12:37 ` [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
@ 2026-01-29  2:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-01-29  2:50 UTC (permalink / raw)
  To: Luis Gerhorst
  Cc: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
	yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, isolodrai,
	memxor, bpf, linux-kernel, linux-kselftest

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Tue, 27 Jan 2026 12:59:10 +0100 you wrote:
> This fixes the verifier_bug_if() that runs on nospec_result to not trigger
> for BPF_CALL (bug reported by Hu, Mei, and Mu). See patch 1 for a full
> description and patch 2 for a test (based on the PoC from the report).
> 
> While working on this I noticed two other problems:
> 
> - nospec_result is currently ignored for BPF_CALL during patching, but it
>   may be required if we assume the CPU may speculate into/out of functions.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] bpf: Fix verifier_bug_if to account for BPF_CALL
    https://git.kernel.org/bpf/bpf-next/c/cd3b6a3d49f8
  - [bpf-next,2/2] bpf: Test nospec after dead stack write in helper
    https://git.kernel.org/bpf/bpf-next/c/60d2c438c1bb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-01-29  2:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-27 11:59 [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
2026-01-27 11:59 ` [PATCH bpf-next 1/2] " Luis Gerhorst
2026-01-27 11:59 ` [PATCH bpf-next 2/2] bpf: Test nospec after dead stack write in helper Luis Gerhorst
2026-01-27 12:37 ` [PATCH bpf-next 0/2] bpf: Fix verifier_bug_if to account for BPF_CALL Luis Gerhorst
2026-01-29  2:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox