Skip to content

Commit

Permalink
libbpf: Don't validate TYPE_ID relo's original imm value
Browse files Browse the repository at this point in the history
During linking, type IDs in the resulting linked BPF object file can
change, and so ldimm64 instructions corresponding to
BPF_CORE_TYPE_ID_TARGET and BPF_CORE_TYPE_ID_LOCAL CO-RE relos can get
their imm value out of sync with actual CO-RE relocation information
that's updated by BPF linker properly during linking process.

We could teach BPF linker to adjust such instructions, but it feels
a bit too much for linker to re-implement good chunk of
bpf_core_patch_insns logic just for this. This is a redundant safety
check for TYPE_ID relocations, as the real validation is in matching
CO-RE specs, so if that works fine, it's very unlikely that there is
something wrong with the instruction itself.

So, instead, teach libbpf (and kernel) to ignore insn->imm for
BPF_CORE_TYPE_ID_TARGET and BPF_CORE_TYPE_ID_LOCAL relos.

Signed-off-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
  • Loading branch information
anakryiko authored and Alexei Starovoitov committed Dec 13, 2021
1 parent f124688 commit 4b443bc
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions tools/lib/bpf/relo_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -709,10 +709,14 @@ static int bpf_core_calc_field_relo(const char *prog_name,

static int bpf_core_calc_type_relo(const struct bpf_core_relo *relo,
const struct bpf_core_spec *spec,
__u32 *val)
__u32 *val, bool *validate)
{
__s64 sz;

/* by default, always check expected value in bpf_insn */
if (validate)
*validate = true;

/* type-based relos return zero when target type is not found */
if (!spec) {
*val = 0;
Expand All @@ -722,6 +726,11 @@ static int bpf_core_calc_type_relo(const struct bpf_core_relo *relo,
switch (relo->kind) {
case BPF_CORE_TYPE_ID_TARGET:
*val = spec->root_type_id;
/* type ID, embedded in bpf_insn, might change during linking,
* so enforcing it is pointless
*/
if (validate)
*validate = false;
break;
case BPF_CORE_TYPE_EXISTS:
*val = 1;
Expand Down Expand Up @@ -861,8 +870,8 @@ static int bpf_core_calc_relo(const char *prog_name,
res->fail_memsz_adjust = true;
}
} else if (core_relo_is_type_based(relo->kind)) {
err = bpf_core_calc_type_relo(relo, local_spec, &res->orig_val);
err = err ?: bpf_core_calc_type_relo(relo, targ_spec, &res->new_val);
err = bpf_core_calc_type_relo(relo, local_spec, &res->orig_val, &res->validate);
err = err ?: bpf_core_calc_type_relo(relo, targ_spec, &res->new_val, NULL);
} else if (core_relo_is_enumval_based(relo->kind)) {
err = bpf_core_calc_enumval_relo(relo, local_spec, &res->orig_val);
err = err ?: bpf_core_calc_enumval_relo(relo, targ_spec, &res->new_val);
Expand Down Expand Up @@ -1213,7 +1222,8 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,

/* TYPE_ID_LOCAL relo is special and doesn't need candidate search */
if (relo->kind == BPF_CORE_TYPE_ID_LOCAL) {
targ_res.validate = true;
/* bpf_insn's imm value could get out of sync during linking */
targ_res.validate = false;
targ_res.poison = false;
targ_res.orig_val = local_spec->root_type_id;
targ_res.new_val = local_spec->root_type_id;
Expand All @@ -1227,7 +1237,6 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
return -EOPNOTSUPP;
}


for (i = 0, j = 0; i < cands->len; i++) {
err = bpf_core_spec_match(local_spec, cands->cands[i].btf,
cands->cands[i].id, cand_spec);
Expand Down

0 comments on commit 4b443bc

Please sign in to comment.