tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'bpf-fix-verifier_bug_if-to-account-for-bpf_call'

Luis Gerhorst says:

====================
bpf: Fix verifier_bug_if to account for BPF_CALL

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

Link: https://patch.msgid.link/20260127115912.3026761-1-luis.gerhorst@fau.de
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Alexei Starovoitov 95dbe214 08a74918

+30 -6
+8 -6
kernel/bpf/verifier.c
··· 21065 21065 * may skip a nospec patched-in after the jump. This can 21066 21066 * currently never happen because nospec_result is only 21067 21067 * used for the write-ops 21068 - * `*(size*)(dst_reg+off)=src_reg|imm32` which must 21069 - * never skip the following insn. Still, add a warning 21070 - * to document this in case nospec_result is used 21071 - * elsewhere in the future. 21068 + * `*(size*)(dst_reg+off)=src_reg|imm32` and helper 21069 + * calls. These must never skip the following insn 21070 + * (i.e., bpf_insn_successors()'s opcode_info.can_jump 21071 + * is false). Still, add a warning to document this in 21072 + * case nospec_result is used elsewhere in the future. 21072 21073 * 21073 21074 * All non-branch instructions have a single 21074 21075 * fall-through edge. For these, nospec_result should 21075 21076 * already work. 21076 21077 */ 21077 - if (verifier_bug_if(BPF_CLASS(insn->code) == BPF_JMP || 21078 - BPF_CLASS(insn->code) == BPF_JMP32, env, 21078 + if (verifier_bug_if((BPF_CLASS(insn->code) == BPF_JMP || 21079 + BPF_CLASS(insn->code) == BPF_JMP32) && 21080 + BPF_OP(insn->code) != BPF_CALL, env, 21079 21081 "speculation barrier after jump instruction may not have the desired effect")) 21080 21082 return -EFAULT; 21081 21083 process_bpf_exit:
+22
tools/testing/selftests/bpf/progs/verifier_unpriv.c
··· 950 950 " ::: __clobber_all); 951 951 } 952 952 953 + SEC("socket") 954 + __description("unpriv: nospec after dead stack write in helper") 955 + __success __success_unpriv 956 + __retval(0) 957 + /* Dead code sanitizer rewrites the call to `goto -1`. */ 958 + __naked void unpriv_dead_helper_stack_write_nospec_result(void) 959 + { 960 + asm volatile (" \ 961 + r0 = 0; \ 962 + if r0 != 1 goto l0_%=; \ 963 + r2 = 0; \ 964 + r3 = r10; \ 965 + r3 += -16; \ 966 + r4 = 4; \ 967 + r5 = 0; \ 968 + call %[bpf_skb_load_bytes_relative]; \ 969 + l0_%=: exit; \ 970 + " : 971 + : __imm(bpf_skb_load_bytes_relative) 972 + : __clobber_all); 973 + } 974 + 953 975 char _license[] SEC("license") = "GPL";