Re: [PATCH bpf-next v4 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section
From: Andrii Nakryiko <hidden>
Date: 2020-09-10 19:41:32
Also in:
bpf
On Wed, Sep 9, 2020 at 11:25 AM Stanislav Fomichev [off-list ref] wrote:
From: YiFei Zhu <redacted> The patch adds a simple wrapper bpf_prog_bind_map around the syscall. When the libbpf tries to load a program, it will probe the kernel for the support of this syscall and unconditionally bind .rodata section
btw, you subject is out of sync, still mentions .metadata
to the program. Cc: YiFei Zhu <redacted> Signed-off-by: YiFei Zhu <redacted>
Please drop zhuyifei@google.com from CC list (it's unreachable), when you submit a new version.
quoted hunk ↗ jump to hunk
Signed-off-by: Stanislav Fomichev <redacted> --- tools/lib/bpf/bpf.c | 13 ++++++ tools/lib/bpf/bpf.h | 8 ++++ tools/lib/bpf/libbpf.c | 94 ++++++++++++++++++++++++++++++++-------- tools/lib/bpf/libbpf.map | 1 + 4 files changed, 98 insertions(+), 18 deletions(-)diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c index 82b983ff6569..5f6c5676cc45 100644 --- a/tools/lib/bpf/bpf.c +++ b/tools/lib/bpf/bpf.c@@ -872,3 +872,16 @@ int bpf_enable_stats(enum bpf_stats_type type) return sys_bpf(BPF_ENABLE_STATS, &attr, sizeof(attr)); } + +int bpf_prog_bind_map(int prog_fd, int map_fd, + const struct bpf_prog_bind_opts *opts) +{ + union bpf_attr attr; +
you forgot OPTS_VALID check here
+ memset(&attr, 0, sizeof(attr)); + attr.prog_bind_map.prog_fd = prog_fd; + attr.prog_bind_map.map_fd = map_fd; + attr.prog_bind_map.flags = OPTS_GET(opts, flags, 0); + + return sys_bpf(BPF_PROG_BIND_MAP, &attr, sizeof(attr)); +}
[...]
quoted hunk ↗ jump to hunk
-static int probe_kern_global_data(void) +static void probe_create_global_data(int *prog, int *map, + struct bpf_insn *insns, size_t insns_cnt) { struct bpf_load_program_attr prg_attr; struct bpf_create_map_attr map_attr; char *cp, errmsg[STRERR_BUFSIZE]; - struct bpf_insn insns[] = { - BPF_LD_MAP_VALUE(BPF_REG_1, 0, 16), - BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 42), - BPF_MOV64_IMM(BPF_REG_0, 0), - BPF_EXIT_INSN(), - }; - int ret, map; + int err; memset(&map_attr, 0, sizeof(map_attr)); map_attr.map_type = BPF_MAP_TYPE_ARRAY;@@ -3748,26 +3749,40 @@ static int probe_kern_global_data(void) map_attr.value_size = 32; map_attr.max_entries = 1; - map = bpf_create_map_xattr(&map_attr); - if (map < 0) { - ret = -errno; - cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg)); + *map = bpf_create_map_xattr(&map_attr); + if (*map < 0) { + err = errno; + cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg)); pr_warn("Error in %s():%s(%d). Couldn't create simple array map.\n", - __func__, cp, -ret); - return ret; + __func__, cp, -err); + return; } - insns[0].imm = map; + insns[0].imm = *map;
I think I already complained about this? You are assuming that insns[0] is BPF_LD_MAP_VALUE, which is true only for one case out of two already! It's just by luck that probe_prog_bind_map works because the verifier ignores the exit code, apparently. If this doesn't generalize well, don't generalize. But let's not do a blind instruction rewrite, which will cause tons of confusion later.
quoted hunk ↗ jump to hunk
memset(&prg_attr, 0, sizeof(prg_attr)); prg_attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER; prg_attr.insns = insns; - prg_attr.insns_cnt = ARRAY_SIZE(insns); + prg_attr.insns_cnt = insns_cnt; prg_attr.license = "GPL"; - ret = bpf_load_program_xattr(&prg_attr, NULL, 0); + *prog = bpf_load_program_xattr(&prg_attr, NULL, 0); +} + +static int probe_kern_global_data(void) +{ + struct bpf_insn insns[] = { + BPF_LD_MAP_VALUE(BPF_REG_1, 0, 16), + BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 42), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }; + int prog = -1, map = -1; + + probe_create_global_data(&prog, &map, insns, ARRAY_SIZE(insns)); + close(map); - return probe_fd(ret); + return probe_fd(prog); } static int probe_kern_btf(void)@@ -3894,6 +3909,32 @@ static int probe_kern_probe_read_kernel(void) return probe_fd(bpf_load_program_xattr(&attr, NULL, 0)); } +static int probe_prog_bind_map(void) +{ + struct bpf_insn insns[] = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }; + int prog = -1, map = -1, ret = 0; + + if (!kernel_supports(FEAT_GLOBAL_DATA)) + return 0; + + probe_create_global_data(&prog, &map, insns, ARRAY_SIZE(insns)); + + if (map >= 0 && prog < 0) { + close(map); + return 0; + } + + if (!bpf_prog_bind_map(prog, map, NULL)) + ret = 1; + + close(map); + close(prog); + return ret; +} +
[...]
quoted hunk ↗ jump to hunk
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index 92ceb48a5ca2..0b7830f4ff8b 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map@@ -308,4 +308,5 @@ LIBBPF_0.2.0 { perf_buffer__epoll_fd; perf_buffer__consume_buffer; xsk_socket__create_shared; + bpf_prog_bind_map;
please keep this list sorted
} LIBBPF_0.1.0; -- 2.28.0.526.ge36021eeef-goog