* [PATCH v2 bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 18:01 ` Song Liu
2019-05-29 17:36 ` [PATCH v2 bpf-next 2/9] libbpf: preserve errno before calling into user callback Andrii Nakryiko
` (8 subsequent siblings)
9 siblings, 1 reply; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
Ensure that size of a section w/ BPF instruction is exactly a multiple
of BPF instruction size.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ca4432f5b067..c6c9d632624a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -349,8 +349,11 @@ static int
bpf_program__init(void *data, size_t size, char *section_name, int idx,
struct bpf_program *prog)
{
- if (size < sizeof(struct bpf_insn)) {
- pr_warning("corrupted section '%s'\n", section_name);
+ const size_t bpf_insn_sz = sizeof(struct bpf_insn);
+
+ if (size == 0 || size % bpf_insn_sz) {
+ pr_warning("corrupted section '%s', size: %zu\n",
+ section_name, size);
return -EINVAL;
}
@@ -376,9 +379,8 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
section_name);
goto errout;
}
- prog->insns_cnt = size / sizeof(struct bpf_insn);
- memcpy(prog->insns, data,
- prog->insns_cnt * sizeof(struct bpf_insn));
+ prog->insns_cnt = size / bpf_insn_sz;
+ memcpy(prog->insns, data, size);
prog->idx = idx;
prog->instances.fds = NULL;
prog->instances.nr = -1;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section
2019-05-29 17:36 ` [PATCH v2 bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
@ 2019-05-29 18:01 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2019-05-29 18:01 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: andrii.nakryiko@gmail.com, netdev@vger.kernel.org,
bpf@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team
> On May 29, 2019, at 10:36 AM, Andrii Nakryiko <andriin@fb.com> wrote:
>
> Ensure that size of a section w/ BPF instruction is exactly a multiple
> of BPF instruction size.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ca4432f5b067..c6c9d632624a 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -349,8 +349,11 @@ static int
> bpf_program__init(void *data, size_t size, char *section_name, int idx,
> struct bpf_program *prog)
> {
> - if (size < sizeof(struct bpf_insn)) {
> - pr_warning("corrupted section '%s'\n", section_name);
> + const size_t bpf_insn_sz = sizeof(struct bpf_insn);
> +
> + if (size == 0 || size % bpf_insn_sz) {
> + pr_warning("corrupted section '%s', size: %zu\n",
> + section_name, size);
> return -EINVAL;
> }
>
> @@ -376,9 +379,8 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
> section_name);
> goto errout;
> }
> - prog->insns_cnt = size / sizeof(struct bpf_insn);
> - memcpy(prog->insns, data,
> - prog->insns_cnt * sizeof(struct bpf_insn));
> + prog->insns_cnt = size / bpf_insn_sz;
> + memcpy(prog->insns, data, size);
> prog->idx = idx;
> prog->instances.fds = NULL;
> prog->instances.nr = -1;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 bpf-next 2/9] libbpf: preserve errno before calling into user callback
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 3/9] libbpf: simplify endianness check Andrii Nakryiko
` (7 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
pr_warning ultimately may call into user-provided callback function,
which can clobber errno value, so we need to save it before that.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c6c9d632624a..5af331cb8e4f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -565,12 +565,12 @@ static int bpf_object__elf_init(struct bpf_object *obj)
} else {
obj->efile.fd = open(obj->path, O_RDONLY);
if (obj->efile.fd < 0) {
- char errmsg[STRERR_BUFSIZE];
- char *cp = libbpf_strerror_r(errno, errmsg,
- sizeof(errmsg));
+ char errmsg[STRERR_BUFSIZE], *cp;
+ err = -errno;
+ cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warning("failed to open %s: %s\n", obj->path, cp);
- return -errno;
+ return err;
}
obj->efile.elf = elf_begin(obj->efile.fd,
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 bpf-next 3/9] libbpf: simplify endianness check
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 2/9] libbpf: preserve errno before calling into user callback Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 4/9] libbpf: check map name retrieved from ELF Andrii Nakryiko
` (6 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
Rewrite endianness check to use "more canonical" way, using
compiler-defined macros, similar to few other places in libbpf. It also
is more obvious and shorter.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 37 ++++++++++++-------------------------
1 file changed, 12 insertions(+), 25 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5af331cb8e4f..fe700ef1135d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -607,31 +607,18 @@ static int bpf_object__elf_init(struct bpf_object *obj)
return err;
}
-static int
-bpf_object__check_endianness(struct bpf_object *obj)
-{
- static unsigned int const endian = 1;
-
- switch (obj->efile.ehdr.e_ident[EI_DATA]) {
- case ELFDATA2LSB:
- /* We are big endian, BPF obj is little endian. */
- if (*(unsigned char const *)&endian != 1)
- goto mismatch;
- break;
-
- case ELFDATA2MSB:
- /* We are little endian, BPF obj is big endian. */
- if (*(unsigned char const *)&endian != 0)
- goto mismatch;
- break;
- default:
- return -LIBBPF_ERRNO__ENDIAN;
- }
-
- return 0;
-
-mismatch:
- pr_warning("Error: endianness mismatch.\n");
+static int bpf_object__check_endianness(struct bpf_object *obj)
+{
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+ if (obj->efile.ehdr.e_ident[EI_DATA] == ELFDATA2LSB)
+ return 0;
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+ if (obj->efile.ehdr.e_ident[EI_DATA] == ELFDATA2MSB)
+ return 0;
+#else
+# error "Unrecognized __BYTE_ORDER__"
+#endif
+ pr_warning("endianness mismatch.\n");
return -LIBBPF_ERRNO__ENDIAN;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 bpf-next 4/9] libbpf: check map name retrieved from ELF
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
` (2 preceding siblings ...)
2019-05-29 17:36 ` [PATCH v2 bpf-next 3/9] libbpf: simplify endianness check Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 5/9] libbpf: fix error code returned on corrupted ELF Andrii Nakryiko
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
Validate there was no error retrieving symbol name corresponding to
a BPF map.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index fe700ef1135d..e3c0144e454f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -920,6 +920,11 @@ bpf_object__init_maps(struct bpf_object *obj, int flags)
map_name = elf_strptr(obj->efile.elf,
obj->efile.strtabidx,
sym.st_name);
+ if (!map_name) {
+ pr_warning("failed to get map #%d name sym string for obj %s\n",
+ map_idx, obj->path);
+ return -LIBBPF_ERRNO__FORMAT;
+ }
obj->maps[map_idx].libbpf_type = LIBBPF_MAP_UNSPEC;
obj->maps[map_idx].offset = sym.st_value;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 bpf-next 5/9] libbpf: fix error code returned on corrupted ELF
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
` (3 preceding siblings ...)
2019-05-29 17:36 ` [PATCH v2 bpf-next 4/9] libbpf: check map name retrieved from ELF Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF Andrii Nakryiko
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
All of libbpf errors are negative, except this one. Fix it.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e3c0144e454f..c972fa10271f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1221,7 +1221,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
if (!obj->efile.strtabidx || obj->efile.strtabidx >= idx) {
pr_warning("Corrupted ELF file: index of strtab invalid\n");
- return LIBBPF_ERRNO__FORMAT;
+ return -LIBBPF_ERRNO__FORMAT;
}
if (btf_data) {
obj->btf = btf__new(btf_data->d_buf, btf_data->d_size);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
` (4 preceding siblings ...)
2019-05-29 17:36 ` [PATCH v2 bpf-next 5/9] libbpf: fix error code returned on corrupted ELF Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-07-02 22:57 ` Stanislav Fomichev
2019-05-29 17:36 ` [PATCH v2 bpf-next 7/9] libbpf: simplify two pieces of logic Andrii Nakryiko
` (3 subsequent siblings)
9 siblings, 1 reply; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
0 is a valid FD, so it's better to initialize it to -1, as is done in
other places. Also, technically, BTF type ID 0 is valid (it's a VOID
type), so it's more reliable to check btf_fd, instead of
btf_key_type_id, to determine if there is any BTF associated with a map.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c972fa10271f..a27a0351e595 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
create_attr.key_size = def->key_size;
create_attr.value_size = def->value_size;
create_attr.max_entries = def->max_entries;
- create_attr.btf_fd = 0;
+ create_attr.btf_fd = -1;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
if (bpf_map_type__is_map_in_map(def->type) &&
@@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
}
*pfd = bpf_create_map_xattr(&create_attr);
- if (*pfd < 0 && create_attr.btf_key_type_id) {
+ if (*pfd < 0 && create_attr.btf_fd >= 0) {
cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
map->name, cp, errno);
- create_attr.btf_fd = 0;
+ create_attr.btf_fd = -1;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
map->btf_key_type_id = 0;
@@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
char *log_buf;
int ret;
+ if (!insns || !insns_cnt)
+ return -EINVAL;
+
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;
@@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
load_attr.license = license;
load_attr.kern_version = kern_version;
load_attr.prog_ifindex = prog->prog_ifindex;
- load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
+ load_attr.prog_btf_fd = prog->btf_fd;
load_attr.func_info = prog->func_info;
load_attr.func_info_rec_size = prog->func_info_rec_size;
load_attr.func_info_cnt = prog->func_info_cnt;
@@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
load_attr.line_info_cnt = prog->line_info_cnt;
load_attr.log_level = prog->log_level;
load_attr.prog_flags = prog->prog_flags;
- if (!load_attr.insns || !load_attr.insns_cnt)
- return -EINVAL;
retry_load:
log_buf = malloc(log_buf_size);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF
2019-05-29 17:36 ` [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF Andrii Nakryiko
@ 2019-07-02 22:57 ` Stanislav Fomichev
2019-07-02 23:18 ` Stanislav Fomichev
0 siblings, 1 reply; 14+ messages in thread
From: Stanislav Fomichev @ 2019-07-02 22:57 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team
On 05/29, Andrii Nakryiko wrote:
> 0 is a valid FD, so it's better to initialize it to -1, as is done in
> other places. Also, technically, BTF type ID 0 is valid (it's a VOID
> type), so it's more reliable to check btf_fd, instead of
> btf_key_type_id, to determine if there is any BTF associated with a map.
>
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c972fa10271f..a27a0351e595 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> create_attr.key_size = def->key_size;
> create_attr.value_size = def->value_size;
> create_attr.max_entries = def->max_entries;
> - create_attr.btf_fd = 0;
> + create_attr.btf_fd = -1;
> create_attr.btf_key_type_id = 0;
> create_attr.btf_value_type_id = 0;
> if (bpf_map_type__is_map_in_map(def->type) &&
> @@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
> }
>
> *pfd = bpf_create_map_xattr(&create_attr);
> - if (*pfd < 0 && create_attr.btf_key_type_id) {
> + if (*pfd < 0 && create_attr.btf_fd >= 0) {
> cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
> pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> map->name, cp, errno);
> - create_attr.btf_fd = 0;
> + create_attr.btf_fd = -1;
This breaks libbpf compatibility with the older kernels. If the kernel
doesn't know about btf_fd and we set it to -1, then CHECK_ATTR
fails :-(
Any objections to converting BTF retries to bpf_capabilities and then
knowingly passing bft_fd==0 or proper fd?
> create_attr.btf_key_type_id = 0;
> create_attr.btf_value_type_id = 0;
> map->btf_key_type_id = 0;
> @@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> char *log_buf;
> int ret;
>
> + if (!insns || !insns_cnt)
> + return -EINVAL;
> +
> 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;
> @@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> load_attr.license = license;
> load_attr.kern_version = kern_version;
> load_attr.prog_ifindex = prog->prog_ifindex;
> - load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
> + load_attr.prog_btf_fd = prog->btf_fd;
> load_attr.func_info = prog->func_info;
> load_attr.func_info_rec_size = prog->func_info_rec_size;
> load_attr.func_info_cnt = prog->func_info_cnt;
> @@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> load_attr.line_info_cnt = prog->line_info_cnt;
> load_attr.log_level = prog->log_level;
> load_attr.prog_flags = prog->prog_flags;
> - if (!load_attr.insns || !load_attr.insns_cnt)
> - return -EINVAL;
>
> retry_load:
> log_buf = malloc(log_buf_size);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF
2019-07-02 22:57 ` Stanislav Fomichev
@ 2019-07-02 23:18 ` Stanislav Fomichev
0 siblings, 0 replies; 14+ messages in thread
From: Stanislav Fomichev @ 2019-07-02 23:18 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team
On 07/02, Stanislav Fomichev wrote:
> On 05/29, Andrii Nakryiko wrote:
> > 0 is a valid FD, so it's better to initialize it to -1, as is done in
> > other places. Also, technically, BTF type ID 0 is valid (it's a VOID
> > type), so it's more reliable to check btf_fd, instead of
> > btf_key_type_id, to determine if there is any BTF associated with a map.
> >
> > Acked-by: Song Liu <songliubraving@fb.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index c972fa10271f..a27a0351e595 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> > create_attr.key_size = def->key_size;
> > create_attr.value_size = def->value_size;
> > create_attr.max_entries = def->max_entries;
> > - create_attr.btf_fd = 0;
> > + create_attr.btf_fd = -1;
> > create_attr.btf_key_type_id = 0;
> > create_attr.btf_value_type_id = 0;
> > if (bpf_map_type__is_map_in_map(def->type) &&
> > @@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
> > }
> >
> > *pfd = bpf_create_map_xattr(&create_attr);
> > - if (*pfd < 0 && create_attr.btf_key_type_id) {
> > + if (*pfd < 0 && create_attr.btf_fd >= 0) {
> > cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
> > pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> > map->name, cp, errno);
> > - create_attr.btf_fd = 0;
> > + create_attr.btf_fd = -1;
> This breaks libbpf compatibility with the older kernels. If the kernel
> doesn't know about btf_fd and we set it to -1, then CHECK_ATTR
> fails :-(
>
> Any objections to converting BTF retries to bpf_capabilities and then
> knowingly passing bft_fd==0 or proper fd?
Oh, nevermind, it looks like you fixed it already in e55d54f43d3f.
> > create_attr.btf_key_type_id = 0;
> > create_attr.btf_value_type_id = 0;
> > map->btf_key_type_id = 0;
> > @@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > char *log_buf;
> > int ret;
> >
> > + if (!insns || !insns_cnt)
> > + return -EINVAL;
> > +
> > 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;
> > @@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > load_attr.license = license;
> > load_attr.kern_version = kern_version;
> > load_attr.prog_ifindex = prog->prog_ifindex;
> > - load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
> > + load_attr.prog_btf_fd = prog->btf_fd;
> > load_attr.func_info = prog->func_info;
> > load_attr.func_info_rec_size = prog->func_info_rec_size;
> > load_attr.func_info_cnt = prog->func_info_cnt;
> > @@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > load_attr.line_info_cnt = prog->line_info_cnt;
> > load_attr.log_level = prog->log_level;
> > load_attr.prog_flags = prog->prog_flags;
> > - if (!load_attr.insns || !load_attr.insns_cnt)
> > - return -EINVAL;
> >
> > retry_load:
> > log_buf = malloc(log_buf_size);
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 bpf-next 7/9] libbpf: simplify two pieces of logic
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
` (5 preceding siblings ...)
2019-05-29 17:36 ` [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 8/9] libbpf: typo and formatting fixes Andrii Nakryiko
` (2 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
Extra check for type is unnecessary in first case.
Extra zeroing is unnecessary, as snprintf guarantees that it will
zero-terminate string.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a27a0351e595..5ea84ab69db1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1430,8 +1430,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
if (maps[map_idx].libbpf_type != type)
continue;
if (type != LIBBPF_MAP_UNSPEC ||
- (type == LIBBPF_MAP_UNSPEC &&
- maps[map_idx].offset == sym.st_value)) {
+ maps[map_idx].offset == sym.st_value) {
pr_debug("relocation: find map %zd (%s) for insn %u\n",
map_idx, maps[map_idx].name, insn_idx);
break;
@@ -2354,7 +2353,6 @@ struct bpf_object *bpf_object__open_buffer(void *obj_buf,
snprintf(tmp_name, sizeof(tmp_name), "%lx-%lx",
(unsigned long)obj_buf,
(unsigned long)obj_buf_sz);
- tmp_name[sizeof(tmp_name) - 1] = '\0';
name = tmp_name;
}
pr_debug("loading object '%s' from buffer\n",
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 bpf-next 8/9] libbpf: typo and formatting fixes
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
` (6 preceding siblings ...)
2019-05-29 17:36 ` [PATCH v2 bpf-next 7/9] libbpf: simplify two pieces of logic Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 17:36 ` [PATCH v2 bpf-next 9/9] libbpf: reduce unnecessary line wrapping Andrii Nakryiko
2019-05-29 23:26 ` [PATCH v2 bpf-next 0/9] libbpf random fixes Daniel Borkmann
9 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
A bunch of typo and formatting fixes.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5ea84ab69db1..6bd7dd544e41 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -505,7 +505,7 @@ static struct bpf_object *bpf_object__new(const char *path,
obj->efile.fd = -1;
/*
- * Caller of this function should also calls
+ * Caller of this function should also call
* bpf_object__elf_finish() after data collection to return
* obj_buf to user. If not, we should duplicate the buffer to
* avoid user freeing them before elf finish.
@@ -574,8 +574,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
}
obj->efile.elf = elf_begin(obj->efile.fd,
- LIBBPF_ELF_C_READ_MMAP,
- NULL);
+ LIBBPF_ELF_C_READ_MMAP, NULL);
}
if (!obj->efile.elf) {
@@ -594,9 +593,9 @@ static int bpf_object__elf_init(struct bpf_object *obj)
ep = &obj->efile.ehdr;
/* Old LLVM set e_machine to EM_NONE */
- if ((ep->e_type != ET_REL) || (ep->e_machine && (ep->e_machine != EM_BPF))) {
- pr_warning("%s is not an eBPF object file\n",
- obj->path);
+ if (ep->e_type != ET_REL ||
+ (ep->e_machine && ep->e_machine != EM_BPF)) {
+ pr_warning("%s is not an eBPF object file\n", obj->path);
err = -LIBBPF_ERRNO__FORMAT;
goto errout;
}
@@ -1438,7 +1437,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
}
if (map_idx >= nr_maps) {
- pr_warning("bpf relocation: map_idx %d large than %d\n",
+ pr_warning("bpf relocation: map_idx %d larger than %d\n",
(int)map_idx, (int)nr_maps - 1);
return -LIBBPF_ERRNO__RELOC;
}
@@ -1797,7 +1796,7 @@ bpf_object__create_maps(struct bpf_object *obj)
}
}
- pr_debug("create map %s: fd=%d\n", map->name, *pfd);
+ pr_debug("created map %s: fd=%d\n", map->name, *pfd);
}
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 bpf-next 9/9] libbpf: reduce unnecessary line wrapping
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
` (7 preceding siblings ...)
2019-05-29 17:36 ` [PATCH v2 bpf-next 8/9] libbpf: typo and formatting fixes Andrii Nakryiko
@ 2019-05-29 17:36 ` Andrii Nakryiko
2019-05-29 23:26 ` [PATCH v2 bpf-next 0/9] libbpf random fixes Daniel Borkmann
9 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:36 UTC (permalink / raw)
To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
There are a bunch of lines of code or comments that are unnecessary
wrapped into multi-lines. Fix that without violating any code
guidelines.
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 52 +++++++++++++-----------------------------
1 file changed, 16 insertions(+), 36 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6bd7dd544e41..9042506acb65 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -497,8 +497,7 @@ static struct bpf_object *bpf_object__new(const char *path,
strcpy(obj->path, path);
/* Using basename() GNU version which doesn't modify arg. */
- strncpy(obj->name, basename((void *)path),
- sizeof(obj->name) - 1);
+ strncpy(obj->name, basename((void *)path), sizeof(obj->name) - 1);
end = strchr(obj->name, '.');
if (end)
*end = 0;
@@ -578,15 +577,13 @@ static int bpf_object__elf_init(struct bpf_object *obj)
}
if (!obj->efile.elf) {
- pr_warning("failed to open %s as ELF file\n",
- obj->path);
+ pr_warning("failed to open %s as ELF file\n", obj->path);
err = -LIBBPF_ERRNO__LIBELF;
goto errout;
}
if (!gelf_getehdr(obj->efile.elf, &obj->efile.ehdr)) {
- pr_warning("failed to get EHDR from %s\n",
- obj->path);
+ pr_warning("failed to get EHDR from %s\n", obj->path);
err = -LIBBPF_ERRNO__FORMAT;
goto errout;
}
@@ -622,18 +619,15 @@ static int bpf_object__check_endianness(struct bpf_object *obj)
}
static int
-bpf_object__init_license(struct bpf_object *obj,
- void *data, size_t size)
+bpf_object__init_license(struct bpf_object *obj, void *data, size_t size)
{
- memcpy(obj->license, data,
- min(size, sizeof(obj->license) - 1));
+ memcpy(obj->license, data, min(size, sizeof(obj->license) - 1));
pr_debug("license of %s is %s\n", obj->path, obj->license);
return 0;
}
static int
-bpf_object__init_kversion(struct bpf_object *obj,
- void *data, size_t size)
+bpf_object__init_kversion(struct bpf_object *obj, void *data, size_t size)
{
__u32 kver;
@@ -643,8 +637,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
}
memcpy(&kver, data, sizeof(kver));
obj->kern_version = kver;
- pr_debug("kernel version of %s is %x\n", obj->path,
- obj->kern_version);
+ pr_debug("kernel version of %s is %x\n", obj->path, obj->kern_version);
return 0;
}
@@ -800,8 +793,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, struct bpf_map *map,
def->key_size = sizeof(int);
def->value_size = data->d_size;
def->max_entries = 1;
- def->map_flags = type == LIBBPF_MAP_RODATA ?
- BPF_F_RDONLY_PROG : 0;
+ def->map_flags = type == LIBBPF_MAP_RODATA ? BPF_F_RDONLY_PROG : 0;
if (data_buff) {
*data_buff = malloc(data->d_size);
if (!*data_buff) {
@@ -816,8 +808,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, struct bpf_map *map,
return 0;
}
-static int
-bpf_object__init_maps(struct bpf_object *obj, int flags)
+static int bpf_object__init_maps(struct bpf_object *obj, int flags)
{
int i, map_idx, map_def_sz = 0, nr_syms, nr_maps = 0, nr_maps_glob = 0;
bool strict = !(flags & MAPS_RELAX_COMPAT);
@@ -1098,8 +1089,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
/* Elf is corrupted/truncated, avoid calling elf_strptr. */
if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL)) {
- pr_warning("failed to get e_shstrndx from %s\n",
- obj->path);
+ pr_warning("failed to get e_shstrndx from %s\n", obj->path);
return -LIBBPF_ERRNO__FORMAT;
}
@@ -1340,8 +1330,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
size_t nr_maps = obj->nr_maps;
int i, nrels;
- pr_debug("collecting relocating info for: '%s'\n",
- prog->section_name);
+ pr_debug("collecting relocating info for: '%s'\n", prog->section_name);
nrels = shdr->sh_size / shdr->sh_entsize;
prog->reloc_desc = malloc(sizeof(*prog->reloc_desc) * nrels);
@@ -1366,9 +1355,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
return -LIBBPF_ERRNO__FORMAT;
}
- if (!gelf_getsym(symbols,
- GELF_R_SYM(rel.r_info),
- &sym)) {
+ if (!gelf_getsym(symbols, GELF_R_SYM(rel.r_info), &sym)) {
pr_warning("relocation: symbol %"PRIx64" not found\n",
GELF_R_SYM(rel.r_info));
return -LIBBPF_ERRNO__FORMAT;
@@ -1817,18 +1804,14 @@ check_btf_ext_reloc_err(struct bpf_program *prog, int err,
if (btf_prog_info) {
/*
* Some info has already been found but has problem
- * in the last btf_ext reloc. Must have to error
- * out.
+ * in the last btf_ext reloc. Must have to error out.
*/
pr_warning("Error in relocating %s for sec %s.\n",
info_name, prog->section_name);
return err;
}
- /*
- * Have problem loading the very first info. Ignore
- * the rest.
- */
+ /* Have problem loading the very first info. Ignore the rest. */
pr_warning("Cannot find %s for main program sec %s. Ignore all %s.\n",
info_name, prog->section_name, info_name);
return 0;
@@ -2032,9 +2015,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
return -LIBBPF_ERRNO__RELOC;
}
- err = bpf_program__collect_reloc(prog,
- shdr, data,
- obj);
+ err = bpf_program__collect_reloc(prog, shdr, data, obj);
if (err)
return err;
}
@@ -2354,8 +2335,7 @@ struct bpf_object *bpf_object__open_buffer(void *obj_buf,
(unsigned long)obj_buf_sz);
name = tmp_name;
}
- pr_debug("loading object '%s' from buffer\n",
- name);
+ pr_debug("loading object '%s' from buffer\n", name);
return __bpf_object__open(name, obj_buf, obj_buf_sz, true, true);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 bpf-next 0/9] libbpf random fixes
2019-05-29 17:36 [PATCH v2 bpf-next 0/9] libbpf random fixes Andrii Nakryiko
` (8 preceding siblings ...)
2019-05-29 17:36 ` [PATCH v2 bpf-next 9/9] libbpf: reduce unnecessary line wrapping Andrii Nakryiko
@ 2019-05-29 23:26 ` Daniel Borkmann
9 siblings, 0 replies; 14+ messages in thread
From: Daniel Borkmann @ 2019-05-29 23:26 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, netdev, bpf, ast, kernel-team
On 05/29/2019 07:36 PM, Andrii Nakryiko wrote:
> This patch set is a collection of unrelated fixes for libbpf.
>
> Patch #1 fixes detection of corrupted BPF section w/ instructions.
> Patch #2 fixes possible errno clobbering.
> Patch #3 simplifies endianness check and brings it in line with few other
> similar checks in libbpf.
> Patch #4 adds check for failed map name retrieval from ELF symbol name.
> Patch #5 fixes return error code to be negative.
> Patch #6 fixes using valid fd (0) as a marker of missing associated BTF.
> Patch #7 removes redundant logic in two places.
> Patch #8 fixes typos in comments and debug output, and fixes formatting.
> Patch #9 unwraps a bunch of multi-line statements and comments.
>
> v1->v2:
> - patch #1 simplifications (Song);
>
>
> Andrii Nakryiko (9):
> libbpf: fix detection of corrupted BPF instructions section
> libbpf: preserve errno before calling into user callback
> libbpf: simplify endianness check
> libbpf: check map name retrieved from ELF
> libbpf: fix error code returned on corrupted ELF
> libbpf: use negative fd to specify missing BTF
> libbpf: simplify two pieces of logic
> libbpf: typo and formatting fixes
> libbpf: reduce unnecessary line wrapping
>
> tools/lib/bpf/libbpf.c | 148 +++++++++++++++++------------------------
> 1 file changed, 60 insertions(+), 88 deletions(-)
>
Applied, thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread