lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180705155718.35b9bdd1@cakuba.netronome.com>
Date:   Thu, 5 Jul 2018 15:57:18 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     alexei.starovoitov@...il.com, oss-drivers@...ronome.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with
 bpftool prog load

On Thu, 5 Jul 2018 10:35:24 +0200, Daniel Borkmann wrote:
> On 07/04/2018 04:54 AM, Jakub Kicinski wrote:
> > Add map parameter to prog load which will allow reuse of existing
> > maps instead of creating new ones.
> > 
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> > Reviewed-by: Quentin Monnet <quentin.monnet@...ronome.com>  
> [...]
> > +
> > +			fd = map_parse_fd(&argc, &argv);
> > +			if (fd < 0)
> > +				goto err_free_reuse_maps;
> > +
> > +			map_replace = reallocarray(map_replace, old_map_fds + 1,
> > +						   sizeof(*map_replace));
> > +			if (!map_replace) {
> > +				p_err("mem alloc failed");
> > +				goto err_free_reuse_maps;  
> 
> Series in general looks good to me. However, above reallocarray() doesn't
> exist and hence build fails, please see below. Is that from newest glibc?
> 
> You probably need some fallback implementation or in general have something
> bpftool internal that doesn't make a bet on its availability.
> 
> # make
> 
> Auto-detecting system features:
> ...                        libbfd: [ on  ]
> ...        disassembler-four-args: [ OFF ]
> 
>   CC       bpf_jit_disasm.o
>   LINK     bpf_jit_disasm
>   CC       bpf_dbg.o
>   LINK     bpf_dbg
>   CC       bpf_asm.o
>   BISON    bpf_exp.yacc.c
>   CC       bpf_exp.yacc.o
>   FLEX     bpf_exp.lex.c
>   CC       bpf_exp.lex.o
>   LINK     bpf_asm
>   DESCEND  bpftool
> 
> Auto-detecting system features:
> ...                        libbfd: [ on  ]
> ...        disassembler-four-args: [ OFF ]
> 
>   CC       map_perf_ring.o
>   CC       xlated_dumper.o
>   CC       perf.o
>   CC       prog.o
> prog.c: In function ‘do_load’:
> prog.c:785:18: warning: implicit declaration of function ‘reallocarray’ [-Wimplicit-function-declaration]
>     map_replace = reallocarray(map_replace, old_map_fds + 1,
>                   ^~~~~~~~~~~~
> prog.c:785:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
>     map_replace = reallocarray(map_replace, old_map_fds + 1,
>                 ^
>   CC       common.o
>   CC       cgroup.o
>   CC       main.o
>   CC       json_writer.o
>   CC       cfg.o
>   CC       map.o
>   CC       jit_disasm.o
>   CC       disasm.o
> 
> Auto-detecting system features:
> ...                        libelf: [ on  ]
> ...                           bpf: [ on  ]
> 
> Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'
>   CC       libbpf.o
>   CC       bpf.o
>   CC       nlattr.o
>   CC       btf.o
>   LD       libbpf-in.o
>   LINK     libbpf.a
>   LINK     bpftool
> prog.o: In function `do_load':
> prog.c:(.text+0x23d): undefined reference to `reallocarray'
> collect2: error: ld returned 1 exit status
> Makefile:89: recipe for target 'bpftool' failed
> make[1]: *** [bpftool] Error 1
> Makefile:99: recipe for target 'bpftool' failed
> make: *** [bpftool] Error 2

Oh no..  Sorry & thanks for catching this.  It would be nice to not
depend on Glibc version defines, in case someone is using a different
library.  Jiong suggested we can just use the feature detection, so I
have something like this:

---

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 0911b00b25cc..20a691659381 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -52,8 +52,8 @@ INSTALL ?= install
 RM ?= rm -f
 
 FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args reallocarray
+FEATURE_DISPLAY = libbfd disassembler-four-args reallocarray
 
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
diff --git a/tools/bpf/bpftool/compat.h b/tools/bpf/bpftool/compat.h
new file mode 100644
index 000000000000..7885cedc9efe
--- /dev/null
+++ b/tools/bpf/bpftool/compat.h
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2018 Netronome Systems, Inc. */
+
+#ifndef __BPF_TOOL_COMPAT_H
+#define __BPF_TOOL_COMPAT_H
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+
+static inline void *reallocarray(void *ptr, size_t nmemb, size_t size)
+{
+	return realloc(ptr, nmemb * size);
+}
+#endif
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 1a9a2aefa014..2106adb73631 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -43,6 +43,7 @@
 #include <linux/kernel.h>
 #include <linux/hashtable.h>
 
+#include "compat.h"
 #include "json_writer.h"
 
 #define ptr_to_u64(ptr)	((__u64)(unsigned long)(ptr))
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index dac9563b5470..0516259be70f 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -14,6 +14,7 @@ FILES=                                          \
          test-libaudit.bin                      \
          test-libbfd.bin                        \
          test-disassembler-four-args.bin        \
+         test-reallocarray.bin			\
          test-liberty.bin                       \
          test-liberty-z.bin                     \
          test-cplus-demangle.bin                \
@@ -204,6 +205,9 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 $(OUTPUT)test-disassembler-four-args.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
 
+$(OUTPUT)test-reallocarray.bin:
+	$(BUILD)
+
 $(OUTPUT)test-liberty.bin:
 	$(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty
 
diff --git a/tools/build/feature/test-reallocarray.c b/tools/build/feature/test-reallocarray.c
new file mode 100644
index 000000000000..8170de35150d
--- /dev/null
+++ b/tools/build/feature/test-reallocarray.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <stdlib.h>
+
+int main(void)
+{
+	return !!reallocarray(NULL, 1, 1);
+}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ