[PATCH bpf-next 0/4] Fix few unrelated small bugs and issues

[PATCH bpf-next 0/4] Fix few unrelated small bugs and issues

[Andrii Nakryiko]

Fix few unrelated issues, found by static analysis tools (performed against
libbpf's Github repo). Also fix compilation warning in bpftool polluting
selftests Makefile output.

Andrii Nakryiko (4):
  libbpf: fix error handling bug in btf_dump__new
  libbpf: simplify BTF initialization logic
  libbpf: fix potential multiplication overflow in mmap() size
    calculation
  bpftool: avoid const discard compilation warning

 tools/bpf/bpftool/jit_disasm.c |  2 +-
 tools/lib/bpf/btf_dump.c       |  1 +
 tools/lib/bpf/libbpf.c         | 21 +++++++--------------
 3 files changed, 9 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH bpf-next 1/4] libbpf: fix error handling bug in btf_dump__new

[Andrii Nakryiko]

Fix missing jump to error handling in btf_dump__new, found by Coverity static
code analysis.

Fixes: 9f81654eebe8 ("libbpf: Expose BTF-to-C type declaration emitting API")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf_dump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 885acebd4396..bd09ed1710f1 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -142,6 +142,7 @@ struct btf_dump *btf_dump__new(const struct btf *btf,
 	if (IS_ERR(d->type_names)) {
 		err = PTR_ERR(d->type_names);
 		d->type_names = NULL;
+		goto err;
 	}
 	d->ident_names = hashmap__new(str_hash_fn, str_equal_fn, NULL);
 	if (IS_ERR(d->ident_names)) {
-- 
2.17.1

[PATCH bpf-next 2/4] libbpf: simplify BTF initialization logic

[Andrii Nakryiko]

Current implementation of bpf_object's BTF initialization is very convoluted
and thus prone to errors. It doesn't have to be like that. This patch
simplifies it significantly.

This code also triggered static analysis issues over logically dead code due
to redundant error checks. This simplification should fix that as well.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 3afaca9bce1d..3b0b88c3377d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2297,16 +2297,18 @@ static int bpf_object__init_btf(struct bpf_object *obj,
 				Elf_Data *btf_data,
 				Elf_Data *btf_ext_data)
 {
-	bool btf_required = bpf_object__is_btf_mandatory(obj);
-	int err = 0;
+	int err = -ENOENT;
 
 	if (btf_data) {
 		obj->btf = btf__new(btf_data->d_buf, btf_data->d_size);
 		if (IS_ERR(obj->btf)) {
+			err = PTR_ERR(obj->btf);
+			obj->btf = NULL;
 			pr_warn("Error loading ELF section %s: %d.\n",
 				BTF_ELF_SEC, err);
 			goto out;
 		}
+		err = 0;
 	}
 	if (btf_ext_data) {
 		if (!obj->btf) {
@@ -2324,18 +2326,9 @@ static int bpf_object__init_btf(struct bpf_object *obj,
 		}
 	}
 out:
-	if (err || IS_ERR(obj->btf)) {
-		if (btf_required)
-			err = err ? : PTR_ERR(obj->btf);
-		else
-			err = 0;
-		if (!IS_ERR_OR_NULL(obj->btf))
-			btf__free(obj->btf);
-		obj->btf = NULL;
-	}
-	if (btf_required && !obj->btf) {
+	if (err && bpf_object__is_btf_mandatory(obj)) {
 		pr_warn("BTF is required, but is missing or corrupted.\n");
-		return err == 0 ? -ENOENT : err;
+		return err;
 	}
 	return 0;
 }
-- 
2.17.1

[PATCH bpf-next 3/4] libbpf: fix potential multiplication overflow in mmap() size calculation

[Andrii Nakryiko]

Prevent potential overflow performed in 32-bit integers, before assigning
result to size_t. Reported by LGTM static analysis.

Fixes: eba9c5f498a1 ("libbpf: Refactor global data map initialization")
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 3b0b88c3377d..fc41c3f2e983 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1283,7 +1283,7 @@ static size_t bpf_map_mmap_sz(const struct bpf_map *map)
 	long page_sz = sysconf(_SC_PAGE_SIZE);
 	size_t map_sz;
 
-	map_sz = roundup(map->def.value_size, 8) * map->def.max_entries;
+	map_sz = (size_t)roundup(map->def.value_size, 8) * map->def.max_entries;
 	map_sz = roundup(map_sz, page_sz);
 	return map_sz;
 }
-- 
2.17.1

[PATCH bpf-next 4/4] bpftool: avoid const discard compilation warning

[Andrii Nakryiko]

Avoid compilation warning in bpftool when assigning disassembler_options by
casting explicitly to non-const pointer.

Fixes: 3ddeac6705ab ("tools: bpftool: use 4 context mode for the NFP disasm")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/bpf/bpftool/jit_disasm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index bfed711258ce..22ef85b0f86c 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -119,7 +119,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 	if (disassembler_options)
-		info.disassembler_options = disassembler_options;
+		info.disassembler_options = (char *)disassembler_options;
 	info.buffer = image;
 	info.buffer_length = len;
 
-- 
2.17.1

Re: [PATCH bpf-next 4/4] bpftool: avoid const discard compilation warning

[Quentin Monnet]

2020-01-16 22:08 UTC-0800 ~ Andrii Nakryiko <andriin@fb.com>
> Avoid compilation warning in bpftool when assigning disassembler_options by
> casting explicitly to non-const pointer.
> 
> Fixes: 3ddeac6705ab ("tools: bpftool: use 4 context mode for the NFP disasm")
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/bpf/bpftool/jit_disasm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
> index bfed711258ce..22ef85b0f86c 100644
> --- a/tools/bpf/bpftool/jit_disasm.c
> +++ b/tools/bpf/bpftool/jit_disasm.c
> @@ -119,7 +119,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
>  	info.arch = bfd_get_arch(bfdf);
>  	info.mach = bfd_get_mach(bfdf);
>  	if (disassembler_options)
> -		info.disassembler_options = disassembler_options;
> +		info.disassembler_options = (char *)disassembler_options;
>  	info.buffer = image;
>  	info.buffer_length = len;
>  
> 

Thanks Andrii,

This fix has been proposed and discussed before:
https://lore.kernel.org/bpf/20190328141652.wssqboyekxmp6tkw@yubo-2/t/#u

I still believe that we should not add the cast.

Best regards,
Quentin

Re: [PATCH bpf-next 0/4] Fix few unrelated small bugs and issues

[Alexei Starovoitov]

On Thu, Jan 16, 2020 at 10:07:57PM -0800, Andrii Nakryiko wrote:
> Fix few unrelated issues, found by static analysis tools (performed against
> libbpf's Github repo). Also fix compilation warning in bpftool polluting
> selftests Makefile output.

Applied first three patches. Thanks

Re: [PATCH bpf-next 4/4] bpftool: avoid const discard compilation warning

[Andrii Nakryiko]

On Fri, Jan 17, 2020 at 7:55 AM Quentin Monnet
<quentin.monnet@netronome.com> wrote:
>
> 2020-01-16 22:08 UTC-0800 ~ Andrii Nakryiko <andriin@fb.com>
> > Avoid compilation warning in bpftool when assigning disassembler_options by
> > casting explicitly to non-const pointer.
> >
> > Fixes: 3ddeac6705ab ("tools: bpftool: use 4 context mode for the NFP disasm")
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  tools/bpf/bpftool/jit_disasm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
> > index bfed711258ce..22ef85b0f86c 100644
> > --- a/tools/bpf/bpftool/jit_disasm.c
> > +++ b/tools/bpf/bpftool/jit_disasm.c
> > @@ -119,7 +119,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> >       info.arch = bfd_get_arch(bfdf);
> >       info.mach = bfd_get_mach(bfdf);
> >       if (disassembler_options)
> > -             info.disassembler_options = disassembler_options;
> > +             info.disassembler_options = (char *)disassembler_options;
> >       info.buffer = image;
> >       info.buffer_length = len;
> >
> >
>
> Thanks Andrii,
>
> This fix has been proposed and discussed before:
> https://lore.kernel.org/bpf/20190328141652.wssqboyekxmp6tkw@yubo-2/t/#u
>
> I still believe that we should not add the cast.

Oh, ok, didn't know that. Seems like Alexei dropped it already.

>
> Best regards,
> Quentin