From 4734b0fefbbf98f8c119eb8344efa19dac82cd2c Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: Sat, 4 Apr 2020 01:14:30 -0400 Subject: libbpf: Initialize *nl_pid so gcc 10 is happy Builds of Fedora's kernel-tools package started to fail with "may be used uninitialized" warnings for nl_pid in bpf_set_link_xdp_fd() and bpf_get_link_xdp_info() on the s390 architecture. Although libbpf_netlink_open() always returns a negative number when it does not set *nl_pid, the compiler does not determine this and thus believes the variable might be used uninitialized. Assuage gcc's fears by explicitly initializing nl_pid. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1807781 Signed-off-by: Jeremy Cline Signed-off-by: Daniel Borkmann Acked-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20200404051430.698058-1-jcline@redhat.com --- tools/lib/bpf/netlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools/lib') diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c index 18b5319025e1..9a14694176de 100644 --- a/tools/lib/bpf/netlink.c +++ b/tools/lib/bpf/netlink.c @@ -142,7 +142,7 @@ static int __bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd, struct ifinfomsg ifinfo; char attrbuf[64]; } req; - __u32 nl_pid; + __u32 nl_pid = 0; sock = libbpf_netlink_open(&nl_pid); if (sock < 0) @@ -288,7 +288,7 @@ int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info, { struct xdp_id_md xdp_id = {}; int sock, ret; - __u32 nl_pid; + __u32 nl_pid = 0; __u32 mask; if (flags & ~XDP_FLAGS_MASK || !info_size) -- cgit From f07cbad29741407ace2a9688548fa93d9cb38df3 Mon Sep 17 00:00:00 2001 From: Andrey Ignatov Date: Mon, 6 Apr 2020 22:09:45 -0700 Subject: libbpf: Fix bpf_get_link_xdp_id flags handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently if one of XDP_FLAGS_{DRV,HW,SKB}_MODE flags is passed to bpf_get_link_xdp_id() and there is a single XDP program attached to ifindex, that program's id will be returned by bpf_get_link_xdp_id() in prog_id argument no matter what mode the program is attached in, i.e. flags argument is not taken into account. For example, if there is a single program attached with XDP_FLAGS_SKB_MODE but user calls bpf_get_link_xdp_id() with flags = XDP_FLAGS_DRV_MODE, that skb program will be returned. Fix it by returning info->prog_id only if user didn't specify flags. If flags is specified then return corresponding mode-specific-field from struct xdp_link_info. The initial error was introduced in commit 50db9f073188 ("libbpf: Add a support for getting xdp prog id on ifindex") and then refactored in 473f4e133a12 so 473f4e133a12 is used in the Fixes tag. Fixes: 473f4e133a12 ("libbpf: Add bpf_get_link_xdp_info() function to get more XDP information") Signed-off-by: Andrey Ignatov Signed-off-by: Daniel Borkmann Acked-by: Toke Høiland-Jørgensen Link: https://lore.kernel.org/bpf/0e9e30490b44b447bb2bebc69c7135e7fe7e4e40.1586236080.git.rdna@fb.com --- tools/lib/bpf/netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/lib') diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c index 9a14694176de..0b709fd10bba 100644 --- a/tools/lib/bpf/netlink.c +++ b/tools/lib/bpf/netlink.c @@ -321,7 +321,7 @@ int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info, static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags) { - if (info->attach_mode != XDP_ATTACHED_MULTI) + if (info->attach_mode != XDP_ATTACHED_MULTI && !flags) return info->prog_id; if (flags & XDP_FLAGS_DRV_MODE) return info->drv_prog_id; -- cgit From 25498a1969bf3687c29c29bbac92821d7a0f8b4a Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Tue, 14 Apr 2020 11:26:45 -0700 Subject: libbpf: Always specify expected_attach_type on program load if supported For some types of BPF programs that utilize expected_attach_type, libbpf won't set load_attr.expected_attach_type, even if expected_attach_type is known from section definition. This was done to preserve backwards compatibility with old kernels that didn't recognize expected_attach_type attribute yet (which was added in 5e43f899b03a ("bpf: Check attach type at prog load time"). But this is problematic for some BPF programs that utilize newer features that require kernel to know specific expected_attach_type (e.g., extended set of return codes for cgroup_skb/egress programs). This patch makes libbpf specify expected_attach_type by default, but also detect support for this field in kernel and not set it during program load. This allows to have a good metadata for bpf_program (e.g., bpf_program__get_extected_attach_type()), but still work with old kernels (for cases where it can work at all). Additionally, due to expected_attach_type being always set for recognized program types, bpf_program__attach_cgroup doesn't have to do extra checks to determine correct attach type, so remove that additional logic. Also adjust section_names selftest to account for this change. More detailed discussion can be found in [0]. [0] https://lore.kernel.org/bpf/20200412003604.GA15986@rdna-mbp.dhcp.thefacebook.com/ Fixes: 5cf1e9145630 ("bpf: cgroup inet skb programs can return 0 to 3") Fixes: 5e43f899b03a ("bpf: Check attach type at prog load time") Reported-by: Andrey Ignatov Signed-off-by: Andrii Nakryiko Signed-off-by: Daniel Borkmann Acked-by: Song Liu Acked-by: Andrey Ignatov Link: https://lore.kernel.org/bpf/20200414182645.1368174-1-andriin@fb.com --- tools/lib/bpf/libbpf.c | 126 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 82 insertions(+), 44 deletions(-) (limited to 'tools/lib') diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index ff9174282a8c..8f480e29a6b0 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -178,6 +178,8 @@ struct bpf_capabilities { __u32 array_mmap:1; /* BTF_FUNC_GLOBAL is supported */ __u32 btf_func_global:1; + /* kernel support for expected_attach_type in BPF_PROG_LOAD */ + __u32 exp_attach_type:1; }; enum reloc_type { @@ -194,6 +196,22 @@ struct reloc_desc { int sym_off; }; +struct bpf_sec_def; + +typedef struct bpf_link *(*attach_fn_t)(const struct bpf_sec_def *sec, + struct bpf_program *prog); + +struct bpf_sec_def { + const char *sec; + size_t len; + enum bpf_prog_type prog_type; + enum bpf_attach_type expected_attach_type; + bool is_exp_attach_type_optional; + bool is_attachable; + bool is_attach_btf; + attach_fn_t attach_fn; +}; + /* * bpf_prog should be a better name but it has been used in * linux/filter.h. @@ -204,6 +222,7 @@ struct bpf_program { char *name; int prog_ifindex; char *section_name; + const struct bpf_sec_def *sec_def; /* section_name with / replaced by _; makes recursive pinning * in bpf_object__pin_programs easier */ @@ -3315,6 +3334,37 @@ static int bpf_object__probe_array_mmap(struct bpf_object *obj) return 0; } +static int +bpf_object__probe_exp_attach_type(struct bpf_object *obj) +{ + struct bpf_load_program_attr attr; + struct bpf_insn insns[] = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }; + int fd; + + memset(&attr, 0, sizeof(attr)); + /* use any valid combination of program type and (optional) + * non-zero expected attach type (i.e., not a BPF_CGROUP_INET_INGRESS) + * to see if kernel supports expected_attach_type field for + * BPF_PROG_LOAD command + */ + attr.prog_type = BPF_PROG_TYPE_CGROUP_SOCK; + attr.expected_attach_type = BPF_CGROUP_INET_SOCK_CREATE; + attr.insns = insns; + attr.insns_cnt = ARRAY_SIZE(insns); + attr.license = "GPL"; + + fd = bpf_load_program_xattr(&attr, NULL, 0); + if (fd >= 0) { + obj->caps.exp_attach_type = 1; + close(fd); + return 1; + } + return 0; +} + static int bpf_object__probe_caps(struct bpf_object *obj) { @@ -3325,6 +3375,7 @@ bpf_object__probe_caps(struct bpf_object *obj) bpf_object__probe_btf_func_global, bpf_object__probe_btf_datasec, bpf_object__probe_array_mmap, + bpf_object__probe_exp_attach_type, }; int i, ret; @@ -4861,7 +4912,12 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt, memset(&load_attr, 0, sizeof(struct bpf_load_program_attr)); load_attr.prog_type = prog->type; - load_attr.expected_attach_type = prog->expected_attach_type; + /* old kernels might not support specifying expected_attach_type */ + if (!prog->caps->exp_attach_type && prog->sec_def && + prog->sec_def->is_exp_attach_type_optional) + load_attr.expected_attach_type = 0; + else + load_attr.expected_attach_type = prog->expected_attach_type; if (prog->caps->name) load_attr.name = prog->name; load_attr.insns = insns; @@ -5062,6 +5118,8 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level) return 0; } +static const struct bpf_sec_def *find_sec_def(const char *sec_name); + static struct bpf_object * __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz, const struct bpf_object_open_opts *opts) @@ -5117,24 +5175,17 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz, bpf_object__elf_finish(obj); bpf_object__for_each_program(prog, obj) { - enum bpf_prog_type prog_type; - enum bpf_attach_type attach_type; - - if (prog->type != BPF_PROG_TYPE_UNSPEC) - continue; - - err = libbpf_prog_type_by_name(prog->section_name, &prog_type, - &attach_type); - if (err == -ESRCH) + prog->sec_def = find_sec_def(prog->section_name); + if (!prog->sec_def) /* couldn't guess, but user might manually specify */ continue; - if (err) - goto out; - bpf_program__set_type(prog, prog_type); - bpf_program__set_expected_attach_type(prog, attach_type); - if (prog_type == BPF_PROG_TYPE_TRACING || - prog_type == BPF_PROG_TYPE_EXT) + bpf_program__set_type(prog, prog->sec_def->prog_type); + bpf_program__set_expected_attach_type(prog, + prog->sec_def->expected_attach_type); + + if (prog->sec_def->prog_type == BPF_PROG_TYPE_TRACING || + prog->sec_def->prog_type == BPF_PROG_TYPE_EXT) prog->attach_prog_fd = OPTS_GET(opts, attach_prog_fd, 0); } @@ -6223,23 +6274,32 @@ void bpf_program__set_expected_attach_type(struct bpf_program *prog, prog->expected_attach_type = type; } -#define BPF_PROG_SEC_IMPL(string, ptype, eatype, is_attachable, btf, atype) \ - { string, sizeof(string) - 1, ptype, eatype, is_attachable, btf, atype } +#define BPF_PROG_SEC_IMPL(string, ptype, eatype, eatype_optional, \ + attachable, attach_btf) \ + { \ + .sec = string, \ + .len = sizeof(string) - 1, \ + .prog_type = ptype, \ + .expected_attach_type = eatype, \ + .is_exp_attach_type_optional = eatype_optional, \ + .is_attachable = attachable, \ + .is_attach_btf = attach_btf, \ + } /* Programs that can NOT be attached. */ #define BPF_PROG_SEC(string, ptype) BPF_PROG_SEC_IMPL(string, ptype, 0, 0, 0, 0) /* Programs that can be attached. */ #define BPF_APROG_SEC(string, ptype, atype) \ - BPF_PROG_SEC_IMPL(string, ptype, 0, 1, 0, atype) + BPF_PROG_SEC_IMPL(string, ptype, atype, true, 1, 0) /* Programs that must specify expected attach type at load time. */ #define BPF_EAPROG_SEC(string, ptype, eatype) \ - BPF_PROG_SEC_IMPL(string, ptype, eatype, 1, 0, eatype) + BPF_PROG_SEC_IMPL(string, ptype, eatype, false, 1, 0) /* Programs that use BTF to identify attach point */ #define BPF_PROG_BTF(string, ptype, eatype) \ - BPF_PROG_SEC_IMPL(string, ptype, eatype, 0, 1, 0) + BPF_PROG_SEC_IMPL(string, ptype, eatype, false, 0, 1) /* Programs that can be attached but attach type can't be identified by section * name. Kept for backward compatibility. @@ -6253,11 +6313,6 @@ void bpf_program__set_expected_attach_type(struct bpf_program *prog, __VA_ARGS__ \ } -struct bpf_sec_def; - -typedef struct bpf_link *(*attach_fn_t)(const struct bpf_sec_def *sec, - struct bpf_program *prog); - static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec, struct bpf_program *prog); static struct bpf_link *attach_tp(const struct bpf_sec_def *sec, @@ -6269,17 +6324,6 @@ static struct bpf_link *attach_trace(const struct bpf_sec_def *sec, static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec, struct bpf_program *prog); -struct bpf_sec_def { - const char *sec; - size_t len; - enum bpf_prog_type prog_type; - enum bpf_attach_type expected_attach_type; - bool is_attachable; - bool is_attach_btf; - enum bpf_attach_type attach_type; - attach_fn_t attach_fn; -}; - static const struct bpf_sec_def section_defs[] = { BPF_PROG_SEC("socket", BPF_PROG_TYPE_SOCKET_FILTER), BPF_PROG_SEC("sk_reuseport", BPF_PROG_TYPE_SK_REUSEPORT), @@ -6713,7 +6757,7 @@ int libbpf_attach_type_by_name(const char *name, continue; if (!section_defs[i].is_attachable) return -EINVAL; - *attach_type = section_defs[i].attach_type; + *attach_type = section_defs[i].expected_attach_type; return 0; } pr_debug("failed to guess attach type based on ELF section name '%s'\n", name); @@ -7542,7 +7586,6 @@ static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec, struct bpf_link * bpf_program__attach_cgroup(struct bpf_program *prog, int cgroup_fd) { - const struct bpf_sec_def *sec_def; enum bpf_attach_type attach_type; char errmsg[STRERR_BUFSIZE]; struct bpf_link *link; @@ -7561,11 +7604,6 @@ bpf_program__attach_cgroup(struct bpf_program *prog, int cgroup_fd) link->detach = &bpf_link__detach_fd; attach_type = bpf_program__get_expected_attach_type(prog); - if (!attach_type) { - sec_def = find_sec_def(bpf_program__title(prog, false)); - if (sec_def) - attach_type = sec_def->attach_type; - } link_fd = bpf_link_create(prog_fd, cgroup_fd, attach_type, NULL); if (link_fd < 0) { link_fd = -errno; -- cgit From 49b452c382da2c2d1ccee1265cbb92da905c82f7 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Tue, 14 Apr 2020 16:50:24 +0200 Subject: libbpf: Fix type of old_fd in bpf_xdp_set_link_opts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'old_fd' parameter used for atomic replacement of XDP programs is supposed to be an FD, but was left as a u32 from an earlier iteration of the patch that added it. It was converted to an int when read, so things worked correctly even with negative values, but better change the definition to correctly reflect the intention. Fixes: bd5ca3ef93cd ("libbpf: Add function to set link XDP fd while specifying old program") Reported-by: David Ahern Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Acked-by: David Ahern Acked-by: Song Liu Link: https://lore.kernel.org/bpf/20200414145025.182163-1-toke@redhat.com --- tools/lib/bpf/libbpf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/lib') diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index 44df1d3e7287..f1dacecb1619 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -458,7 +458,7 @@ struct xdp_link_info { struct bpf_xdp_set_link_opts { size_t sz; - __u32 old_fd; + int old_fd; }; #define bpf_xdp_set_link_opts__last_field old_fd -- cgit