On Thu, May 13, 2021 at 5:36 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> From: Alexei Starovoitov <ast@xxxxxxxxxx>
>
> The BPF program loading process performed by libbpf is quite complex
> and consists of the following steps:
> "open" phase:
> - parse elf file and remember relocations, sections
> - collect externs and ksyms including their btf_ids in prog's BTF
> - patch BTF datasec (since llvm couldn't do it)
> - init maps (old style map_def, BTF based, global data map, kconfig map)
> - collect relocations against progs and maps
> "load" phase:
> - probe kernel features
> - load vmlinux BTF
> - resolve externs (kconfig and ksym)
> - load program BTF
> - init struct_ops
> - create maps
> - apply CO-RE relocations
> - patch ld_imm64 insns with src_reg=PSEUDO_MAP, PSEUDO_MAP_VALUE, PSEUDO_BTF_ID
> - reposition subprograms and adjust call insns
> - sanitize and load progs
>
> During this process libbpf does sys_bpf() calls to load BTF, create maps,
> populate maps and finally load programs.
> Instead of actually doing the syscalls generate a trace of what libbpf
> would have done and represent it as the "loader program".
> The "loader program" consists of single map with:
> - union bpf_attr(s)
> - BTF bytes
> - map value bytes
> - insns bytes
> and single bpf program that passes bpf_attr(s) and data into bpf_sys_bpf() helper.
> Executing such "loader program" via bpf_prog_test_run() command will
> replay the sequence of syscalls that libbpf would have done which will result
> the same maps created and programs loaded as specified in the elf file.
> The "loader program" removes libelf and majority of libbpf dependency from
> program loading process.
>
> kconfig, typeless ksym, struct_ops and CO-RE are not supported yet.
>
> The order of relocate_data and relocate_calls had to change, so that
> bpf_gen__prog_load() can see all relocations for a given program with
> correct insn_idx-es.
>
> Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
> tools/lib/bpf/Build | 2 +-
> tools/lib/bpf/bpf_gen_internal.h | 40 ++
> tools/lib/bpf/gen_loader.c | 689 +++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.c | 226 ++++++++--
> tools/lib/bpf/libbpf.h | 12 +
> tools/lib/bpf/libbpf.map | 1 +
> tools/lib/bpf/libbpf_internal.h | 2 +
> tools/lib/bpf/skel_internal.h | 123 ++++++
> 8 files changed, 1060 insertions(+), 35 deletions(-)
> create mode 100644 tools/lib/bpf/bpf_gen_internal.h
> create mode 100644 tools/lib/bpf/gen_loader.c
> create mode 100644 tools/lib/bpf/skel_internal.h
>
[...]
> +void bpf_gen__prog_load(struct bpf_gen *gen,
> + struct bpf_prog_load_params *load_attr, int prog_idx)
> +{
> + int attr_size = offsetofend(union bpf_attr, fd_array);
> + int prog_load_attr, license, insns, func_info, line_info;
> + union bpf_attr attr;
> +
> + memset(&attr, 0, attr_size);
> + pr_debug("gen: prog_load: type %d insns_cnt %zd\n",
> + load_attr->prog_type, load_attr->insn_cnt);
> + /* add license string to blob of bytes */
> + license = add_data(gen, load_attr->license, strlen(load_attr->license) + 1);
> + /* add insns to blob of bytes */
> + insns = add_data(gen, load_attr->insns,
> + load_attr->insn_cnt * sizeof(struct bpf_insn));
> +
> + attr.prog_type = load_attr->prog_type;
> + attr.expected_attach_type = load_attr->expected_attach_type;
> + attr.attach_btf_id = load_attr->attach_btf_id;
> + attr.prog_ifindex = load_attr->prog_ifindex;
> + attr.kern_version = 0;
> + attr.insn_cnt = (__u32)load_attr->insn_cnt;
> + attr.prog_flags = load_attr->prog_flags;
> +
> + attr.func_info_rec_size = load_attr->func_info_rec_size;
> + attr.func_info_cnt = load_attr->func_info_cnt;
> + func_info = add_data(gen, load_attr->func_info,
> + attr.func_info_cnt * attr.func_info_rec_size);
> +
> + attr.line_info_rec_size = load_attr->line_info_rec_size;
> + attr.line_info_cnt = load_attr->line_info_cnt;
> + line_info = add_data(gen, load_attr->line_info,
> + attr.line_info_cnt * attr.line_info_rec_size);
> +
Hey Alexei,
Coverity ([0] and [1]) is complaining that load_attr->func_info and
load_attr->line_info can be NULL in some cases, which will lead to
NULL deref. I'm not sure if we restrict gen_loader to be only used
with BPF applications that have BTF embedded. If not, then it will
cause a crash, so we need to protect against that. Please take a look.
[0] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874059&defectInstanceId=10901198&mergedDefectId=349034
[1] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874059&defectInstanceId=10901191&mergedDefectId=349033
Not sure why we have two issues above, they both look identical, but
for completeness I included both.
> + memcpy(attr.prog_name, load_attr->name,
> + min((unsigned)strlen(load_attr->name), BPF_OBJ_NAME_LEN - 1));
> + prog_load_attr = add_data(gen, &attr, attr_size);
> +
> + /* populate union bpf_attr with a pointer to license */
> + emit_rel_store(gen, attr_field(prog_load_attr, license), license);
> +
[...]
