mirror of
https://github.com/torvalds/linux.git
synced 2024-11-23 20:51:44 +00:00
libbpf: improve handling of unresolved kfuncs
Currently, libbpf leaves `call #0` instruction for __weak unresolved kfuncs, which might lead to a confusing verifier log situations, where invalid `call #0` will be treated as successfully validated. We can do better. Libbpf already has an established mechanism of poisoning instructions that failed some form of resolution (e.g., CO-RE relocation and BPF map set to not be auto-created). Libbpf doesn't fail them outright to allow users to guard them through other means, and as long as BPF verifier can prove that such poisoned instructions cannot be ever reached, this doesn't consistute an invalid BPF program. If user didn't guard such code, libbpf will extract few pieces of information to tie such poisoned instructions back to additional information about what entitity wasn't resolved (e.g., BPF map name, or CO-RE relocation information). __weak unresolved kfuncs fit this model well, so this patch extends libbpf with poisioning and log fixup logic for kfunc calls. Note, this poisoning is done only for kfunc *calls*, not kfunc address resolution (ldimm64 instructions). The former cannot be ever valid, if reached, so it's safe to poison them. The latter is a valid mechanism to check if __weak kfunc ksym was resolved, and do necessary guarding and work arounds based on this result, supported in most recent kernels. As such, libbpf keeps such ldimm64 instructions as loading zero, never poisoning them. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/r/20230418002148.3255690-4-andrii@kernel.org Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
parent
f709160d17
commit
05b6f766b2
@ -5841,6 +5841,30 @@ static void poison_map_ldimm64(struct bpf_program *prog, int relo_idx,
|
||||
}
|
||||
}
|
||||
|
||||
/* unresolved kfunc call special constant, used also for log fixup logic */
|
||||
#define POISON_CALL_KFUNC_BASE 2002000000
|
||||
#define POISON_CALL_KFUNC_PFX "2002"
|
||||
|
||||
static void poison_kfunc_call(struct bpf_program *prog, int relo_idx,
|
||||
int insn_idx, struct bpf_insn *insn,
|
||||
int ext_idx, const struct extern_desc *ext)
|
||||
{
|
||||
pr_debug("prog '%s': relo #%d: poisoning insn #%d that calls kfunc '%s'\n",
|
||||
prog->name, relo_idx, insn_idx, ext->name);
|
||||
|
||||
/* we turn kfunc call into invalid helper call with identifiable constant */
|
||||
insn->code = BPF_JMP | BPF_CALL;
|
||||
insn->dst_reg = 0;
|
||||
insn->src_reg = 0;
|
||||
insn->off = 0;
|
||||
/* if this instruction is reachable (not a dead code),
|
||||
* verifier will complain with something like:
|
||||
* invalid func unknown#2001000123
|
||||
* where lower 123 is extern index into obj->externs[] array
|
||||
*/
|
||||
insn->imm = POISON_CALL_KFUNC_BASE + ext_idx;
|
||||
}
|
||||
|
||||
/* Relocate data references within program code:
|
||||
* - map references;
|
||||
* - global variable references;
|
||||
@ -5913,9 +5937,9 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
|
||||
if (ext->is_set) {
|
||||
insn[0].imm = ext->ksym.kernel_btf_id;
|
||||
insn[0].off = ext->ksym.btf_fd_idx;
|
||||
} else { /* unresolved weak kfunc */
|
||||
insn[0].imm = 0;
|
||||
insn[0].off = 0;
|
||||
} else { /* unresolved weak kfunc call */
|
||||
poison_kfunc_call(prog, i, relo->insn_idx, insn,
|
||||
relo->ext_idx, ext);
|
||||
}
|
||||
break;
|
||||
case RELO_SUBPROG_ADDR:
|
||||
@ -7052,6 +7076,39 @@ static void fixup_log_missing_map_load(struct bpf_program *prog,
|
||||
patch_log(buf, buf_sz, log_sz, line1, line3 - line1, patch);
|
||||
}
|
||||
|
||||
static void fixup_log_missing_kfunc_call(struct bpf_program *prog,
|
||||
char *buf, size_t buf_sz, size_t log_sz,
|
||||
char *line1, char *line2, char *line3)
|
||||
{
|
||||
/* Expected log for failed and not properly guarded kfunc call:
|
||||
* line1 -> 123: (85) call unknown#2002000345
|
||||
* line2 -> invalid func unknown#2002000345
|
||||
* line3 -> <anything else or end of buffer>
|
||||
*
|
||||
* "123" is the index of the instruction that was poisoned.
|
||||
* "345" in "2002000345" is an extern index in obj->externs to fetch kfunc name.
|
||||
*/
|
||||
struct bpf_object *obj = prog->obj;
|
||||
const struct extern_desc *ext;
|
||||
int insn_idx, ext_idx;
|
||||
char patch[128];
|
||||
|
||||
if (sscanf(line1, "%d: (%*d) call unknown#%d\n", &insn_idx, &ext_idx) != 2)
|
||||
return;
|
||||
|
||||
ext_idx -= POISON_CALL_KFUNC_BASE;
|
||||
if (ext_idx < 0 || ext_idx >= obj->nr_extern)
|
||||
return;
|
||||
ext = &obj->externs[ext_idx];
|
||||
|
||||
snprintf(patch, sizeof(patch),
|
||||
"%d: <invalid kfunc call>\n"
|
||||
"kfunc '%s' is referenced but wasn't resolved\n",
|
||||
insn_idx, ext->name);
|
||||
|
||||
patch_log(buf, buf_sz, log_sz, line1, line3 - line1, patch);
|
||||
}
|
||||
|
||||
static void fixup_verifier_log(struct bpf_program *prog, char *buf, size_t buf_sz)
|
||||
{
|
||||
/* look for familiar error patterns in last N lines of the log */
|
||||
@ -7089,6 +7146,15 @@ static void fixup_verifier_log(struct bpf_program *prog, char *buf, size_t buf_s
|
||||
fixup_log_missing_map_load(prog, buf, buf_sz, log_sz,
|
||||
prev_line, cur_line, next_line);
|
||||
return;
|
||||
} else if (str_has_pfx(cur_line, "invalid func unknown#"POISON_CALL_KFUNC_PFX)) {
|
||||
prev_line = find_prev_line(buf, cur_line);
|
||||
if (!prev_line)
|
||||
continue;
|
||||
|
||||
/* reference to unresolved kfunc */
|
||||
fixup_log_missing_kfunc_call(prog, buf, buf_sz, log_sz,
|
||||
prev_line, cur_line, next_line);
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user