History

bpf: Fix s16 truncation for large bpf-to-bpf call offsets

Currently, the BPF instruction set allows bpf-to-bpf calls (or internal calls, pseudo calls) to use a 32-bit imm field to represent the relative jump offset. However, when JIT is disabled or falls back to the interpreter, the verifier invokes bpf_patch_call_args() to rewrite the call instruction. In this function, the 32-bit imm is downcast to s16 and stored in the off field. void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth) { stack_depth = max_t(u32, stack_depth, 1); insn->off = (s16) insn->imm; insn->imm = interpreters_args[(round_up(stack_depth, 32) / 32) - 1] - __bpf_call_base_args; insn->code = BPF_JMP | BPF_CALL_ARGS; } If the original imm exceeds the s16 range (i.e., a jump offset greater than 32767 instructions), this downcast silently truncates the offset, resulting in an incorrect call target. Fix this by: 1. In bpf_patch_call_args(), keeping the imm field unchanged and using the off field to store the index of the interpreter function. 2. In ___bpf_prog_run() for the JMP_CALL_ARGS case, retrieving the interpreter function pointer from the interpreters_args array using the off field as the index, and passing the original imm to calculate the last argument of the interpreter function. After these changes, the truncation issue is resolved, and __bpf_call_base_args is also no longer needed and can be removed, which makes the code cleaner. Performance: In ___bpf_prog_run() for the JMP_CALL_ARGS case, changing the retrieval of the interpreter function pointer from pointer addition to direct array indexing improves performance. The possible reason is that the latter has better instruction-level parallelism. See the v5 discussion [1] for more details. [1] https://lore.kernel.org/bpf/f120c3c4-6999-414a-b514-518bb64b4758@zju.edu.cn/ To avoid requiring bpftool changes, keep the new imm/off encoding internal and restore the legacy xlated dump layout in bpf_insn_prepare_dump(). For bpf-to-bpf call offsets that do not fit in s16, export off as 0 instead of a truncated and misleading value.

Yazhou Tang committed 6 months ago

Commit 58a8f3e1 parent 4314a445 files · +49 −13Browse files

Showing the changes to kernel (3 of 5 files). Show all

Unified · Split

3 files
  1. kernel/bpf/core.c +14 −7
  2. kernel/bpf/fixups.c +3 −3
  3. kernel/bpf/syscall.c +26 −0
kernel/bpf/core.c+14−7
@@ -1771,6 +1771,9 @@ static u32 abs_s32(s32 x) · 1770 unmodified lines
17711771 return x >= 0 ? (u32)x : -(u32)x;
17721772 }
17731773
1774+static u64 (*interpreters_args[])(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5,
1775+ const struct bpf_insn *insn);
1776+
17741777 /**
17751778 * ___bpf_prog_run - run eBPF program on a given context
17761779 * @regs: is the array of MAX_BPF_EXT_REG eBPF pseudo-registers
@@ -2077,10 +2080,9 @@ select_insn: · 300 unmodified lines
20772080 CONT;
20782081
20792082 JMP_CALL_ARGS:
2080− BPF_R0 = (__bpf_call_base_args + insn->imm)(BPF_R1, BPF_R2,
2081− BPF_R3, BPF_R4,
2082− BPF_R5,
2083− insn + insn->off + 1);
2083+ BPF_R0 = interpreters_args[insn->off](BPF_R1, BPF_R2, BPF_R3,
2084+ BPF_R4, BPF_R5,
2085+ insn + insn->imm + 1);
20842086 CONT;
20852087
20862088 JMP_TAIL_CALL: {
@@ -2400,12 +2402,17 @@ int bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth) · 313 unmodified lines
24002402 /* Prevent out-of-bounds read to interpreters_args */
24012403 if (stack_depth > MAX_BPF_STACK)
24022404 return -EINVAL;
2403− insn->off = (s16) insn->imm;
2404− insn->imm = interpreters_args[(round_up(stack_depth, 32) / 32) - 1] -
2405− __bpf_call_base_args;
2405+ insn->off = (round_up(stack_depth, 32) / 32) - 1;
24062406 insn->code = BPF_JMP | BPF_CALL_ARGS;
24072407 return 0;
24082408 }
2409+
2410+s32 bpf_call_args_imm(s16 idx)
2411+{
2412+ if (WARN_ON_ONCE(idx < 0 || idx >= ARRAY_SIZE(interpreters_args)))
2413+ return 0;
2414+ return BPF_CALL_IMM(interpreters_args[idx]);
2415+}
24092416 #endif
24102417 #endif
24112418
kernel/bpf/fixups.c+3−3
@@ -1250,9 +1250,9 @@ static int jit_subprogs(struct bpf_verifier_env *env) · 1249 unmodified lines
12501250 }
12511251 if (!bpf_pseudo_call(insn))
12521252 continue;
1253− insn->off = env->insn_aux_data[i].call_imm;
1254− subprog = bpf_find_subprog(env, i + insn->off + 1);
1255− insn->imm = subprog;
1253+ insn->imm = env->insn_aux_data[i].call_imm;
1254+ subprog = bpf_find_subprog(env, i + insn->imm + 1);
1255+ insn->off = subprog;
12561256 }
12571257
12581258 prog->jited = 1;
kernel/bpf/syscall.c+26−0
@@ -4919,6 +4919,29 @@ out: · 4918 unmodified lines
49194919 return map;
49204920 }
49214921
4922+static void prepare_dump_pseudo_call(struct bpf_insn *insn)
4923+{
4924+ s32 call_off = insn->imm;
4925+
4926+ /*
4927+ * BPF_CALL_ARGS only exists for interpreter fallback.
4928+ * 1. For interpreter (BPF_CALL_ARGS): insn->off is the index of
4929+ * interpreters_args array, so here using bpf_call_args_imm()
4930+ * to get the real address offset.
4931+ * 2. For JIT (BPF_CALL): insn->off is the subprog id.
4932+ */
4933+ if (insn->code == (BPF_JMP | BPF_CALL_ARGS))
4934+ insn->imm = bpf_call_args_imm(insn->off);
4935+ else
4936+ insn->imm = insn->off;
4937+
4938+ /* Avoid dumping a truncated and misleading pc-relative offset. */
4939+ if (call_off > S16_MAX || call_off < S16_MIN)
4940+ insn->off = 0;
4941+ else
4942+ insn->off = call_off;
4943+}
4944+
49224945 static struct bpf_insn *bpf_insn_prepare_dump(const struct bpf_prog *prog,
49234946 const struct cred *f_cred)
49244947 {
@@ -4944,6 +4967,9 @@ static struct bpf_insn *bpf_insn_prepare_dump(const struct bpf_prog *prog, · 19 unmodified lines
49444967 }
49454968 if (code == (BPF_JMP | BPF_CALL) ||
49464969 code == (BPF_JMP | BPF_CALL_ARGS)) {
4970+ /* Restore the legacy xlated dump layout. */
4971+ if (insns[i].src_reg == BPF_PSEUDO_CALL)
4972+ prepare_dump_pseudo_call(&insns[i]);
49474973 if (code == (BPF_JMP | BPF_CALL_ARGS))
49484974 insns[i].code = BPF_JMP | BPF_CALL;
49494975 if (!bpf_dump_raw_ok(f_cred))