On Wed, May 14, 2025 at 10:54 AM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
> static void emit_stx(u8 **pprog, u32 size, u32 dst_reg, u32 src_reg, int off)
> {
> @@ -2010,13 +2037,19 @@ st: if (is_imm8(insn->off))
> case BPF_LDX | BPF_PROBE_MEM32 | BPF_H:
> case BPF_LDX | BPF_PROBE_MEM32 | BPF_W:
> case BPF_LDX | BPF_PROBE_MEM32 | BPF_DW:
> + case BPF_LDX | BPF_PROBE_MEM32SX | BPF_B:
> + case BPF_LDX | BPF_PROBE_MEM32SX | BPF_H:
> + case BPF_LDX | BPF_PROBE_MEM32SX | BPF_W:
> case BPF_STX | BPF_PROBE_MEM32 | BPF_B:
> case BPF_STX | BPF_PROBE_MEM32 | BPF_H:
> case BPF_STX | BPF_PROBE_MEM32 | BPF_W:
> case BPF_STX | BPF_PROBE_MEM32 | BPF_DW:
> start_of_ldx = prog;
> if (BPF_CLASS(insn->code) == BPF_LDX)
> - emit_ldx_r12(&prog, BPF_SIZE(insn->code), dst_reg, src_reg, insn->off);
> + if (BPF_MODE(insn->code) == BPF_PROBE_MEM32SX)
> + emit_ldsx_r12(&prog, BPF_SIZE(insn->code), dst_reg, src_reg, insn->off);
> + else
> + emit_ldx_r12(&prog, BPF_SIZE(insn->code), dst_reg, src_reg, insn->off);
> else
> emit_stx_r12(&prog, BPF_SIZE(insn->code), dst_reg, src_reg, insn->off);
> populate_extable:
Luckily I didn't trust CI and decided to test it manually:
./test_progs-cpuv4 -t arena_spin
[ 68.977751] mem32 extable bug
[ 68.984388] mem32 extable bug
[ 69.182864] mem32 extable bug
[ 69.190027] mem32 extable bug
[ 69.408629] mem32 extable bug
[ 69.415651] mem32 extable bug
libbpf: prog 'prog': BPF program load failed: -EINVAL
libbpf: prog 'prog': -- BEGIN PROG LOAD LOG --
Func#1 ('arena_spin_lock_slowpath') is safe for any args that match
its prototype
calling kernel functions are not allowed in non-JITed programs
processed 408 insns (limit 1000000) max_states_per_insn 1 total_states
42 peak_states 42 mark_read 7
-- END PROG LOAD LOG --
The verifier error is wrong.
The prog failed to JIT, but jit_subprog didn't return EFAULT
and the verifier tried to guess the error with:
if (has_kfunc_call) {
verbose(env, "calling kernel functions are not allowed
in non-JITed programs\n");
return -EINVAL;
}
and guessed it wrong,
but that is a separate issue.
The patch needs this fix:
index 70152200cc8c..a66c288dd812 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21188,6 +21188,7 @@ static int jit_subprogs(struct bpf_verifier_env *env)
if (BPF_CLASS(insn->code) == BPF_LDX &&
(BPF_MODE(insn->code) == BPF_PROBE_MEM ||
BPF_MODE(insn->code) == BPF_PROBE_MEM32 ||
+ BPF_MODE(insn->code) == BPF_PROBE_MEM32SX ||
BPF_MODE(insn->code) == BPF_PROBE_MEMSX))
num_exentries++;
if ((BPF_CLASS(insn->code) == BPF_STX ||
Before I tested it I thought we can apply this patch without
a new selftest, but that would have been a mistake.
We would have landed a half working sign extending loads :(
Please respin with the selftest.
pw-bot: cr
