diff options
author | Ivan Nardi <12729895+IvanNardi@users.noreply.github.com> | 2022-12-05 10:21:42 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-05 10:21:42 +0100 |
commit | 3e4ab39b528e43db8fefdaa542a91260bd100ab2 (patch) | |
tree | 68c6cf4fa9d87056f14532e543ad5a4ef17dafa1 | |
parent | b9f63458e69011aea9aabdd3ffe157d0d4531cc2 (diff) |
Add support for LTO and Gold linker (#1812)
This commit add (optional) support for Link-Time-Optimization and Gold
linker.
This is the first, mandatory step needed to make nDPI compliant with
"introspector" sanitizer requirements in OSS-Fuzz: see
https://github.com/google/oss-fuzz/issues/8939
Gold linker is not supported by Windows and by macOS, so this feature is
disabled by default. It has been enable in CI in two linux targets
("latest" gcc and clang).
Fix some warnings triggered by LTO.
The changes in `src/lib/ndpi_serializer.c` seams reasonable.
However, the change in `tests/unit/unit.c` is due to the following
warning, which seems to be a false positive.
```
unit.c: In function ‘serializerUnitTest’:
ndpi_serializer.c:2258:13: error: ‘MEM[(struct ndpi_private_serializer *)&deserializer].buffer.size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
unit.c:67:31: note: ‘MEM[(struct ndpi_private_serializer *)&deserializer].buffer.size’ was declared here
67 | ndpi_serializer serializer, deserializer;
| ^
ndpi_serializer.c:2605:10: error: ‘MEM[(struct ndpi_private_serializer *)&deserializer].status.buffer.size_used’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
unit.c:67:31: note: ‘MEM[(struct ndpi_private_serializer *)&deserializer].status.buffer.size_used’ was declared here
67 | ndpi_serializer serializer, deserializer;
```
Since this warning is triggered only with an old version of gcc and
`tests/unit/unit.c` is used only during the tests, the easiest fix has
been applied.
Some (unknown to me) combinations of OS and compiler trigger the
following warnings at linker time (with sanitizer and gold linker)
```
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_load1_asm'
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_load2_asm'
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_load4_asm'
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_load8_asm'
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_load16_asm'
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_store1_asm'
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_store2_asm'
/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_report_store4_asm'
[..]
```
I have not found any references to this kind of message, with the only
exception of https://sourceware.org/bugzilla/show_bug.cgi?id=25975
which seems to suggest that these messages can be safely ignored.
In any case, the compilation results are sound.
Fix `clean` target in the Makefile in the `example` directory.
In OSS-Fuzz enviroments, `fuzz_ndpi_reader` reports a strange link error
(as always, when the gold linker is involved...).
It's come out that the culprit was the `tempnam` function: the code has
been changed to use `tmpfile` instead. No sure why... :(
Fuzzing target `fuzz_ndpi_reader.c` doesn't use `libndpiReader.a`
anymore: this way we can use `--with-only-libndpi` flag on Oss-Fuzz builds
as workaround for the "missing dependencies errors" described in
https://github.com/google/oss-fuzz/issues/8939
-rw-r--r-- | .github/workflows/build.yml | 29 | ||||
-rw-r--r-- | configure.ac | 9 | ||||
-rw-r--r-- | example/Makefile.in | 2 | ||||
-rw-r--r-- | fuzz/Makefile.am | 14 | ||||
-rw-r--r-- | fuzz/fuzz_ndpi_reader.c | 39 | ||||
-rw-r--r-- | src/lib/ndpi_serializer.c | 9 | ||||
-rw-r--r-- | tests/unit/unit.c | 2 |
7 files changed, 53 insertions, 51 deletions
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index af417bd60..19da4fe4a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -171,7 +171,7 @@ jobs: git diff-index --quiet HEAD -- || true test: - name: ${{ matrix.os }} ${{ matrix.arch }} ${{ matrix.gcrypt }} ${{ matrix.compiler }} ${{ matrix.pcre }} ${{ matrix.maxminddb }} ${{ matrix.msan }} ${{ matrix.nBPF }} + name: ${{ matrix.os }} ${{ matrix.arch }} ${{ matrix.gcrypt }} ${{ matrix.compiler }} ${{ matrix.pcre }} ${{ matrix.maxminddb }} ${{ matrix.msan }} ${{ matrix.nBPF }} ${{matrix.lto_gold_linker}} runs-on: ${{ matrix.os }} env: CC: ${{ matrix.compiler }} @@ -183,10 +183,13 @@ jobs: arch: ["x86_64"] gcrypt: ["--with-local-libgcrypt", ""] compiler: ["cc"] + ar: ["ar"] + ranlib: ["ranlib"] pcre: [""] maxminddb: [""] msan: [""] nBPF: [""] + lto_gold_linker: [""] include: - compiler: "gcc-7" # "Oldest" gcc easily available os: ubuntu-20.04 @@ -204,6 +207,7 @@ jobs: maxminddb: "--with-maxminddb" msan: "--with-sanitizer" nBPF: "" + lto_gold_linker: "--with-lto-and-gold-linker" - compiler: "clang-7" # "Oldest" clang easily available os: ubuntu-20.04 arch: "x86_64" @@ -213,6 +217,8 @@ jobs: msan: "--with-sanitizer" nBPF: "" - compiler: "clang-14" # "Newest" clang easily available + ar: "llvm-ar-14" + ranlib: "llvm-ranlib-14" os: ubuntu-22.04 arch: "x86_64" gcrypt: "" @@ -220,6 +226,7 @@ jobs: maxminddb: "--with-maxminddb" msan: "--with-sanitizer" nBPF: "" + lto_gold_linker: "--with-lto-and-gold-linker" - compiler: "cc" os: ubuntu-latest arch: "x86_64" @@ -336,12 +343,8 @@ jobs: ./configure make cd - - - name: Setup Ubuntu specified compiler (gcc) - if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.arch, 'x86_64') && startsWith(matrix.compiler, 'gcc') - run: | - sudo apt-get install ${{ matrix.compiler }} - - name: Setup Ubuntu specified compiler (clang) - if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.arch, 'x86_64') && startsWith(matrix.compiler, 'clang') + - name: Setup Ubuntu specified compiler + if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.arch, 'x86_64') && ! startsWith(matrix.compiler, 'cc') run: | sudo apt-get install ${{ matrix.compiler }} - name: Install Windows msys2 prerequisites @@ -390,9 +393,9 @@ jobs: run: | brew install libmaxminddb - name: Configure nDPI on Ubuntu - if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.arch, 'x86_64') && startsWith(matrix.compiler, 'cc') + if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.arch, 'x86_64') run: | - ./autogen.sh --enable-option-checking=fatal --enable-debug-messages ${{ matrix.gcrypt }} ${{ matrix.msan }} ${{ matrix.pcre }} ${{ matrix.maxminddb }} --enable-tls-sigs + AR=${{ matrix.ar }} RANLIB=${{ matrix.ranlib }} ./autogen.sh --enable-option-checking=fatal --enable-debug-messages ${{ matrix.gcrypt }} ${{ matrix.msan }} ${{ matrix.pcre }} ${{ matrix.maxminddb }} --enable-tls-sigs ${{matrix.lto_gold_linker}} - name: Configure nDPI on MacOS if: startsWith(matrix.os, 'macOS') && startsWith(matrix.arch, 'x86_64') && startsWith(matrix.compiler, 'cc') run: | @@ -406,14 +409,6 @@ jobs: run: | msys2 -c 'make all' msys2 -c 'ldd ./example/ndpiReader.exe' - - name: Configure nDPI with specified GCC version on Ubuntu - if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.arch, 'x86_64') && startsWith(matrix.compiler, 'gcc') - run: | - ./autogen.sh --enable-option-checking=fatal --enable-debug-messages ${{ matrix.gcrypt }} ${{ matrix.msan }} ${{ matrix.pcre }} ${{ matrix.maxminddb }} --enable-tls-sigs - - name: Configure nDPI with specified CLANG on Ubuntu - if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.arch, 'x86_64') && startsWith(matrix.compiler, 'clang') - run: | - ./autogen.sh --enable-option-checking=fatal --enable-debug-messages ${{ matrix.gcrypt }} ${{ matrix.msan }} ${{ matrix.pcre }} ${{ matrix.maxminddb }} --enable-tls-sigs - name: Build nDPI if: startsWith(matrix.arch, 'x86_64') && !startsWith(matrix.os, 'windows') run: | diff --git a/configure.ac b/configure.ac index fbf1bb8c2..3b3e64d4c 100644 --- a/configure.ac +++ b/configure.ac @@ -26,6 +26,7 @@ AC_ARG_WITH(local-libgcrypt, AS_HELP_STRING([--with-local-libgcrypt], [Build wi AC_ARG_ENABLE(tls-sigs, AS_HELP_STRING([--enable-tls-sigs], [Enable TLS Client signature algorithm dissection. Rarely used, but requires significantly more memory.])) AC_ARG_ENABLE(npcap, AS_HELP_STRING([--disable-npcap], [msys2 only: Disable linkage against the wpcap/npcap import library in windows/WpdPack/Lib.])) AC_ARG_WITH(nbpf-path, AS_HELP_STRING([--with-nbpf-path], [nBPF library custom path; default: ${PWD}/../PF_RING/userland/nbpf]),[NBPF_HOME=$withval],[NBPF_HOME=${PWD}/../PF_RING/userland/nbpf]) +AC_ARG_WITH(lto-and-gold-linker, AS_HELP_STRING([--with-lto-and-gold-linker], [Build with LTO and Gold linker])]) AS_IF([test "x$enable_fuzztargets" = "xyes"], [BUILD_FUZZTARGETS=1], [BUILD_FUZZTARGETS=0]) AM_CONDITIONAL([BUILD_FUZZTARGETS], [test "x$enable_fuzztargets" = "xyes"]) @@ -42,7 +43,8 @@ AS_IF([test "${with_thread_sanitizer+set}" = set -a "${with_memory_sanitizer+set AS_IF([test "${with_sanitizer+set}" = set -o "${with_thread_sanitizer+set}" = set -o "${with_memory_sanitizer+set}" = set],[ NDPI_CFLAGS="${NDPI_CFLAGS} -O0 -g3" ],[ - NDPI_CFLAGS="${NDPI_CFLAGS} -O2" + dnl> Oss-fuzz doesn't really like any optimizaton flags don't set by itself + AS_IF([test "x$enable_fuzztargets" != "xyes"], [NDPI_CFLAGS="${NDPI_CFLAGS} -O2"]) ]) AS_IF([test "${with_sanitizer+set}" = set -o "${with_thread_sanitizer+set}" = set -o "${with_memory_sanitizer+set}" = set],[ AS_IF([test "x$enable_gprof" = "xyes"], [ @@ -167,6 +169,11 @@ AS_IF([test "x${enable_tls_sigs}" = "xyes"],[ NDPI_CFLAGS="-W -Wall -Wno-unused-parameter -Wno-unused-function -Wno-address-of-packed-member ${NDPI_CFLAGS}" +AS_IF([test "${with_lto_and_gold_linker+set}" = set], [ + NDPI_CFLAGS="${NDPI_CFLAGS} -flto -fuse-ld=gold -Wno-unused-command-line-argument" + NDPI_LDFLAGS="${NDPI_LDFLAGS} ${NDPI_CFLAGS}" +]) + AC_CHECK_HEADERS([netinet/in.h stdint.h stdlib.h string.h unistd.h math.h float.h]) AC_CHECK_LIB([m], [sqrt], [], [LIBM="-lm"]) AC_CHECK_LIB([rrd], [rrd_fetch_r], [LIBRRD=-lrrd]) diff --git a/example/Makefile.in b/example/Makefile.in index 73bc51986..d6dbaa21a 100644 --- a/example/Makefile.in +++ b/example/Makefile.in @@ -79,7 +79,7 @@ cppcheck: cppcheck --template='{file}:{line}:{severity}:{message}' --quiet --enable=all --force -I$(SRCHOME)/include *.c clean: - /bin/rm -f *.o ndpiReader ndpiSimpleIntegration ndpiReader$(EXE_SUFFIX) ndpiSimpleIntegration$(EXE_SUFFIX) ndpiReader.dpdk + /bin/rm -f *.o ndpiReader ndpiSimpleIntegration ndpiReader$(EXE_SUFFIX) ndpiSimpleIntegration$(EXE_SUFFIX) ndpiReader.dpdk libndpiReader.a /bin/rm -f .*.dpdk.cmd .*.o.cmd *.dpdk.map .*.o.d /bin/rm -f _install _postbuild _postinstall _preinstall /bin/rm -rf build diff --git a/fuzz/Makefile.am b/fuzz/Makefile.am index 6b9a090a6..d739fb00a 100644 --- a/fuzz/Makefile.am +++ b/fuzz/Makefile.am @@ -1,7 +1,7 @@ bin_PROGRAMS = fuzz_process_packet fuzz_ndpi_reader fuzz_quic_get_crypto_data fuzz_process_packet_SOURCES = fuzz_process_packet.c -fuzz_process_packet_CFLAGS = +fuzz_process_packet_CFLAGS = @NDPI_CFLAGS@ $(CXXFLAGS) fuzz_process_packet_LDADD = ../src/lib/libndpi.a fuzz_process_packet_LDFLAGS = $(ADDITIONAL_LIBS) $(LIBS) if HAS_FUZZLDFLAGS @@ -13,9 +13,9 @@ fuzz_process_packet_LINK=$(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \ $(LIBTOOLFLAGS) --mode=link $(CXX) @NDPI_CFLAGS@ $(AM_CXXFLAGS) $(CXXFLAGS) \ $(fuzz_process_packet_LDFLAGS) @NDPI_LDFLAGS@ $(LDFLAGS) -o $@ -fuzz_ndpi_reader_SOURCES = fuzz_ndpi_reader.c -fuzz_ndpi_reader_CFLAGS = -I../example/ -fuzz_ndpi_reader_LDADD = ../example/libndpiReader.a ../src/lib/libndpi.a +fuzz_ndpi_reader_SOURCES = fuzz_ndpi_reader.c ../example/reader_util.c +fuzz_ndpi_reader_CFLAGS = -I../example/ @NDPI_CFLAGS@ $(CXXFLAGS) +fuzz_ndpi_reader_LDADD = ../src/lib/libndpi.a fuzz_ndpi_reader_LDFLAGS = $(PCAP_LIB) $(ADDITIONAL_LIBS) $(LIBS) if HAS_FUZZLDFLAGS fuzz_ndpi_reader_CFLAGS += $(LIB_FUZZING_ENGINE) @@ -27,9 +27,9 @@ fuzz_ndpi_reader_LINK=$(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \ $(fuzz_ndpi_reader_LDFLAGS) @NDPI_LDFLAGS@ $(LDFLAGS) -o $@ fuzz_quic_get_crypto_data_SOURCES = fuzz_quic_get_crypto_data.c -fuzz_quic_get_crypto_data_CFLAGS = -I../example/ -fuzz_quic_get_crypto_data_LDADD = ../example/libndpiReader.a ../src/lib/libndpi.a -fuzz_quic_get_crypto_data_LDFLAGS = $(PCAP_LIB) $(ADDITIONAL_LIBS) $(LIBS) +fuzz_quic_get_crypto_data_CFLAGS = @NDPI_CFLAGS@ $(CXXFLAGS) +fuzz_quic_get_crypto_data_LDADD = ../src/lib/libndpi.a +fuzz_quic_get_crypto_data_LDFLAGS = $(ADDITIONAL_LIBS) $(LIBS) if HAS_FUZZLDFLAGS fuzz_quic_get_crypto_data_CFLAGS += $(LIB_FUZZING_ENGINE) fuzz_quic_get_crypto_data_LDFLAGS += $(LIB_FUZZING_ENGINE) diff --git a/fuzz/fuzz_ndpi_reader.c b/fuzz/fuzz_ndpi_reader.c index 9f75a69f7..1adba5939 100644 --- a/fuzz/fuzz_ndpi_reader.c +++ b/fuzz/fuzz_ndpi_reader.c @@ -23,25 +23,20 @@ int malloc_size_stats = 0; int max_malloc_bins = 0; struct ndpi_bin malloc_bins; /* unused */ -int bufferToFile(const char * name, const uint8_t *Data, size_t Size) { - FILE * fd; - if (remove(name) != 0) { - if (errno != ENOENT) { - perror("remove failed"); - return -1; - } - } - fd = fopen(name, "wb"); +FILE *bufferToFile(const uint8_t *Data, size_t Size) { + FILE *fd; + fd = tmpfile(); if (fd == NULL) { - perror("open failed"); - return -2; + perror("Error tmpfile"); + return NULL; } if (fwrite (Data, 1, Size, fd) != Size) { + perror("Error fwrite"); fclose(fd); - return -3; + return NULL; } - fclose(fd); - return 0; + rewind(fd); + return fd; } int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { @@ -51,8 +46,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { int r; char errbuf[PCAP_ERRBUF_SIZE]; NDPI_PROTOCOL_BITMASK all; - char * pcap_path = tempnam("/tmp", "fuzz-ndpi-reader"); u_int i; + FILE *fd; if (prefs == NULL) { prefs = calloc(sizeof(struct ndpi_workflow_prefs), 1); @@ -78,20 +73,19 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { ndpi_finalize_initialization(workflow->ndpi_struct); } - bufferToFile(pcap_path, Data, Size); + fd = bufferToFile(Data, Size); + if (fd == NULL) + return 0; - pkts = pcap_open_offline(pcap_path, errbuf); + pkts = pcap_fopen_offline(fd, errbuf); if (pkts == NULL) { - remove(pcap_path); - free(pcap_path); + fclose(fd); return 0; } if (ndpi_is_datalink_supported(pcap_datalink(pkts)) == 0) { /* Do not fail if the datalink type is not supported (may happen often during fuzzing). */ pcap_close(pkts); - remove(pcap_path); - free(pcap_path); return 0; } @@ -122,8 +116,5 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { ndpi_tdestroy(workflow->ndpi_flows_root[i], ndpi_flow_info_freer); ndpi_free(workflow->ndpi_flows_root); - remove(pcap_path); - free(pcap_path); - return 0; } diff --git a/src/lib/ndpi_serializer.c b/src/lib/ndpi_serializer.c index 85096c100..f383d471c 100644 --- a/src/lib/ndpi_serializer.c +++ b/src/lib/ndpi_serializer.c @@ -2404,6 +2404,8 @@ int ndpi_deserialize_key_string(ndpi_deserializer *_deserializer, int size; expected = sizeof(u_int8_t) /* type */; + key->str = NULL; + key->str_len = 0; if(buff_diff < expected) return(-2); kt = ndpi_deserialize_get_key_subtype(deserializer); @@ -2429,6 +2431,7 @@ int ndpi_deserialize_value_uint32(ndpi_deserializer *_deserializer, int size; expected = sizeof(u_int8_t) /* type */; + *value = 0; if(buff_diff < expected) return(-2); kt = ndpi_deserialize_get_key_subtype(deserializer); @@ -2512,6 +2515,7 @@ int ndpi_deserialize_value_int32(ndpi_deserializer *_deserializer, int size; expected = sizeof(u_int8_t) /* type */; + *value = 0; if(buff_diff < expected) return(-2); kt = ndpi_deserialize_get_key_subtype(deserializer); @@ -2558,6 +2562,7 @@ int ndpi_deserialize_value_int64(ndpi_deserializer *_deserializer, int rc; expected = sizeof(u_int8_t) /* type */; + *value = 0; if(buff_diff < expected) return(-2); kt = ndpi_deserialize_get_key_subtype(deserializer); @@ -2593,6 +2598,7 @@ int ndpi_deserialize_value_float(ndpi_deserializer *_deserializer, int size; expected = sizeof(u_int8_t) /* type */; + *value = 0; if(buff_diff < expected) return(-2); kt = ndpi_deserialize_get_key_subtype(deserializer); @@ -2624,6 +2630,7 @@ int ndpi_deserialize_value_double(ndpi_deserializer *_deserializer, int size; expected = sizeof(u_int8_t) /* type */; + *value = 0; if(buff_diff < expected) return(-2); kt = ndpi_deserialize_get_key_subtype(deserializer); @@ -2656,6 +2663,8 @@ int ndpi_deserialize_value_string(ndpi_deserializer *_deserializer, int size; expected = sizeof(u_int8_t) /* type */; + value->str = NULL; + value->str_len = 0; if(buff_diff < expected) return(-2); kt = ndpi_deserialize_get_key_subtype(deserializer); diff --git a/tests/unit/unit.c b/tests/unit/unit.c index 3efb5624b..184d1bdc6 100644 --- a/tests/unit/unit.c +++ b/tests/unit/unit.c @@ -64,7 +64,7 @@ static int verbose = 0; #define FLT_MAX 3.402823466e+38F int serializerUnitTest() { - ndpi_serializer serializer, deserializer; + ndpi_serializer serializer = {0}, deserializer = {0}; int i, loop_id; ndpi_serialization_format fmt = {0}; u_int32_t buffer_len; |