From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id CDBC2469710 for ; Tue, 26 May 2020 19:02:29 +0300 (MSK) Date: Tue, 26 May 2020 18:54:04 +0300 From: Igor Munkin Message-ID: <20200526155404.GN5455@tarantool.org> References: <33ba1d50585e101aa025d899116c48d56be59202.1587372354.git.sergeyb@tarantool.org> <20200428111901.GQ11314@tarantool.org> <20200429120956.GB83580@pony.bronevichok.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200429120956.GB83580@pony.bronevichok.ru> Subject: Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Bronnikov Cc: o.piskunov@tarantool.org, tarantool-patches@dev.tarantool.org, alexander.turenko@tarantool.org Sergey, Thanks for the changes! Please consider my replies below. On 29.04.20, Sergey Bronnikov wrote: > Hi, Igor > > thanks a lot for review! See my answers inline. > > On 14:19 Tue 28 Apr , Igor Munkin wrote: > > Sergey, > > > > Thanks for the patch! I left several comments, please consider them. > > > > On 20.04.20, Sergey Bronnikov wrote: > > > - introduce CMake flag to enable building fuzzers > > > - add fuzzers based on LibFuzzer[1] to csv, http_parser and uri modules > > > > > > [1] https://llvm.org/docs/LibFuzzer.html > > > > > > Note: LibFuzzer requires Clang compiler > > > > > > How-To Use: > > > > > > $ mkdir build && cd build > > > $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON .. > > > $ make -j > > > $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4 I guess corpus directory need to be created prior to the run. At least I faced the following error: | $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4 | INFO: Seed: 3782841313 | INFO: Loaded 1 modules (3 inline 8-bit counters): 3 [0x574eb0, | 0x574eb3), | INFO: Loaded 1 PC tables (3 PCs): 3 [0x54f540,0x54f570), | No such file or directory: corpus/; exiting After everything works fine. > > > > I can't test your patch manually since this recipe doesn't work for me: > > perhaps you should install package sys-libs/compiler-rt-sanitizers [1] This package was installed. | $ $ eix sys-libs/compiler-rt-sanitizers | head -1 | [I] sys-libs/compiler-rt-sanitizers > > 1. https://packages.gentoo.org/packages/sys-libs/compiler-rt-sanitizers > > > | $ cat /etc/os-release > > | NAME=Gentoo > > | ID=gentoo > > | PRETTY_NAME="Gentoo/Linux" > > | ANSI_COLOR="1;32" > > | HOME_URL="https://www.gentoo.org/" > > | SUPPORT_URL="https://www.gentoo.org/support/" > > | BUG_REPORT_URL="https://bugs.gentoo.org/" > > | $ clang --version > > | clang version 8.0.1 (tags/RELEASE_801/final) > > | Target: x86_64-pc-linux-gnu > > | Thread model: posix > > | InstalledDir: /usr/lib/llvm/8/bin > > | $ clang++ --version > > | clang version 8.0.1 (tags/RELEASE_801/final) > > | Target: x86_64-pc-linux-gnu > > | Thread model: posix > > | InstalledDir: /usr/lib/llvm/8/bin > > | $ git status | head -1 > > | On branch ligurio/gh-1809-libfuzzer > > | $ mkdir -p libfuzzer > > | $ cd !$ > > | cd libfuzzer > > | $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON .. > > | -- Enabling LTO: FALSE > > | -- OpenSSL 1.1.1d found > > | -- > > | CMake Deprecation Warning at test/CMakeLists.txt:21 (cmake_policy): > > | The OLD behavior for policy CMP0037 will be removed from a future version > > | of CMake. > > | > > | The cmake-policies(7) manual explains that the OLD behaviors of all > > | policies are deprecated and that a policy should be set to OLD only under > > | specific short-term circumstances. Projects should be ported to the NEW > > | behavior and not rely on setting a policy to OLD. > > | > > | > > | -- > > | -- > > | -- Configuring done > > | -- Generating done > > | -- Build files have been written to: /home/imun/projects/tarantool > > | $ echo $? > > | 0 > > | $ make -j > > | make: *** No targets specified and no makefile found. Stop. > > I suspect the problem was due to specifying "CXX=clang++". > I have changed check for compiler and it can probably fixes your problem. Yeah, this might be the root cause. As I mentioned above, now everything is fine, thanks! > > > > > diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c > > > new file mode 100644 > > > index 000000000..aa1703514 > > > --- /dev/null > > > +++ b/src/lib/csv/test_csv.c > > > > This C file doesn't respect our C style guide[1], please adjust it. > > > > > @@ -0,0 +1,23 @@ > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include "csv.h" > > > + > > > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { > > > + struct csv csv; > > > + csv_create(&csv); > > > + char *buf = calloc(size, sizeof(char*)); > > > + if (buf == NULL) { > > > + return -1; > > > + } > > > + memcpy(buf, data, size); > > > + buf[size] = '\0'; > > > + char *end = buf + size; > > > + csv_parse_chunk(&csv, buf, end); > > > + csv_finish_parsing(&csv); > > > + csv_destroy(&csv); > > > + free(buf); > > > + > > > + return 0; > > > > So, csv parsing succeeds regardless the input? Then what is the purpose > > of the testing? > > Yes, function LLVMFuzzerTestOneInput succeeds when csv parsing > finishes without any issues like memory corruption, buffer overflow etc. > Unfortunately these csv_* functions returns void and I cannot additionaly > check was parsing successfull or not. Anyway fuzzing is about negative testing > and our goal to check function for reliability with junk input. No, you can. Despite the void function return type you can check the error_status field in csv context structure which is set e.g. in routine (called within routine). > > > > +} > > > diff --git a/src/lib/http_parser/CMakeLists.txt b/src/lib/http_parser/CMakeLists.txt > > > index a48f83cb6..7af922fbb 100644 > > > --- a/src/lib/http_parser/CMakeLists.txt > > > +++ b/src/lib/http_parser/CMakeLists.txt > > > @@ -1 +1,12 @@ > > > add_library(http_parser STATIC http_parser.c) > > > + > > > +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang") > > > + set(TestName "test_http_parser") > > > + add_executable(${TestName} ${TestName}.c) > > > + set_target_properties(${TestName} > > > + PROPERTIES > > > + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1" > > > + LINK_FLAGS "-fsanitize=fuzzer,address") > > > + target_link_libraries(${TestName} PRIVATE http_parser) > > > + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}") > > > > What if ${TESTING_OUTPUT_DIRECTORY} is not defined? > > Why it can be undefined if I set it in cmake/profile.cmake when ENABLE_FUZZER is enabled? Yes, but if one runs the following command, the build succeeds: | $ cmake . && make -j | -- The C compiler identification is GNU 8.3.0 | -- The CXX compiler identification is GNU 8.3.0 | -- Check for working C compiler: /usr/bin/cc | -- Check for working C compiler: /usr/bin/cc -- works | -- Detecting C compiler ABI info | -- Detecting C compiler ABI info - done | -- Detecting C compile features | -- Detecting C compile features - done | -- Check for working CXX compiler: /usr/bin/c++ | -- Check for working CXX compiler: /usr/bin/c++ -- works | -- Detecting CXX compiler ABI info | -- Detecting CXX compiler ABI info - done | -- Detecting CXX compile features | -- Detecting CXX compile features - done | CMake Warning (dev) in CMakeLists.txt: | No cmake_minimum_required command is present. A line of code such as | | cmake_minimum_required(VERSION 3.14) | | should be added at the top of the file. The version specified may be lower | if you wish to support older CMake versions for this project. For more | information run "cmake --help-policy CMP0000". | This warning is for project developers. Use -Wno-dev to suppress it. | | -- Configuring done | -- Generating done | -- Build files have been written to: /src/lib/http_parser | Scanning dependencies of target http_parser | [ 50%] Building C object CMakeFiles/http_parser.dir/http_parser.o | [100%] Linking C static library libhttp_parser.a | [100%] Built target http_parser At the same time the following command fails due to invalid compiler/linker flags: | $ cmake -DENABLE_FUZZER=ON . && make -j | -- The C compiler identification is GNU 8.3.0 | -- The CXX compiler identification is GNU 8.3.0 | -- Check for working C compiler: /usr/bin/cc | -- Check for working C compiler: /usr/bin/cc -- works | -- Detecting C compiler ABI info | -- Detecting C compiler ABI info - done | -- Detecting C compile features | -- Detecting C compile features - done | -- Check for working CXX compiler: /usr/bin/c++ | -- Check for working CXX compiler: /usr/bin/c++ -- works | -- Detecting CXX compiler ABI info | -- Detecting CXX compiler ABI info - done | -- Detecting CXX compile features | -- Detecting CXX compile features - done | CMake Warning (dev) in CMakeLists.txt: | No cmake_minimum_required command is present. A line of code such as | | cmake_minimum_required(VERSION 3.14) | | should be added at the top of the file. The version specified may be lower | if you wish to support older CMake versions for this project. For more | information run "cmake --help-policy CMP0000". | This warning is for project developers. Use -Wno-dev to suppress it. | | -- Configuring done | -- Generating done | -- Build files have been written to: /src/lib/http_parser | Scanning dependencies of target http_parser | [ 25%] Building C object CMakeFiles/http_parser.dir/http_parser.o | [ 50%] Linking C static library libhttp_parser.a | [ 50%] Built target http_parser | Scanning dependencies of target test_http_parser | [ 75%] Building C object CMakeFiles/test_http_parser.dir/test_http_parser.o | cc: error: unrecognized argument to -fsanitize= option: 'fuzzer' | make[2]: *** [CMakeFiles/test_http_parser.dir/build.make:63: CMakeFiles/test_http_parser.dir/test_http_parser.o] Error 1 | make[1]: *** [CMakeFiles/Makefile2:110: CMakeFiles/test_http_parser.dir/all] Error 2 | make: *** [Makefile:84: all] Error 2 So cmake respects the given options but has no check for toolchain. OK, when the proper toolchain is set command succeeds and test_http_parser is build in the project root directory: | $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON . && make -j | -- The C compiler identification is Clang 8.0.1 | -- The CXX compiler identification is Clang 8.0.1 | -- Check for working C compiler: /usr/lib/llvm/8/bin/clang | -- Check for working C compiler: /usr/lib/llvm/8/bin/clang -- works | -- Detecting C compiler ABI info | -- Detecting C compiler ABI info - done | -- Detecting C compile features | -- Detecting C compile features - done | -- Check for working CXX compiler: /usr/lib/llvm/8/bin/clang++ | -- Check for working CXX compiler: /usr/lib/llvm/8/bin/clang++ -- works | -- Detecting CXX compiler ABI info | -- Detecting CXX compiler ABI info - done | -- Detecting CXX compile features | -- Detecting CXX compile features - done | CMake Warning (dev) in CMakeLists.txt: | No cmake_minimum_required command is present. A line of code such as | | cmake_minimum_required(VERSION 3.14) | | should be added at the top of the file. The version specified may be lower | if you wish to support older CMake versions for this project. For more | information run "cmake --help-policy CMP0000". | This warning is for project developers. Use -Wno-dev to suppress it. | | -- Configuring done | -- Generating done | -- Build files have been written to: /src/lib/http_parser | Scanning dependencies of target http_parser | [ 25%] Building C object CMakeFiles/http_parser.dir/http_parser.o | [ 50%] Linking C static library libhttp_parser.a | [ 50%] Built target http_parser | Scanning dependencies of target test_http_parser | [ 75%] Building C object CMakeFiles/test_http_parser.dir/test_http_parser.o | [100%] Linking C executable test_http_parser | [100%] Built target test_http_parser And this is the case I was writing about. Do you consider this as a bug? Side note: this *might* be the main reason, why unit tests aren't placed alongside with the corresponding libs but grouped within a separate directory as Serge mentioned in his review. > > > -- > sergeyb@ -- Best regards, IM