[PATCH libnbd INCOMPLETE 0/3] fuzzing: Add and use fuzzed data provider
As the subject implies, this is incomplete because I'm still having conceptual problems with how to make it work. Posting to the list because there's some interesting code & ideas here. Rich.
In a future commit we will change the fuzzing technique to use a fuzzed data provider. After this, test cases will no longer contain the straight output from nbdkit. Therefore the --write option will no longer be useful. (We will need to write a test case by hand instead) --- fuzzing/libnbd-fuzz-wrapper.c | 176 +--------------------------------- 1 file changed, 3 insertions(+), 173 deletions(-) diff --git a/fuzzing/libnbd-fuzz-wrapper.c b/fuzzing/libnbd-fuzz-wrapper.c index 23ef131..338adc0 100644 --- a/fuzzing/libnbd-fuzz-wrapper.c +++ b/fuzzing/libnbd-fuzz-wrapper.c @@ -42,29 +42,11 @@ #include <libnbd.h> #ifndef SOCK_CLOEXEC -/* We do use exec, but only when using --write, which is a maintainer - * operation that always runs on Linux. - */ -#define SOCK_CLOEXEC 0 +#define SOCK_CLOEXEC 0 /* This file doesn't use exec */ #endif -/* If defined, instead of creating the fuzzer wrapper, write a working - * testcase to the named file. This runs nbdkit and captures the - * input to libnbd (output from nbdkit) in the file. Use it like - * this: - * - * Generate test case: - * $ fuzzing/libnbd-fuzz-wrapper --write testcase - * Test it: - * $ LIBNBD_DEBUG=1 fuzzing/libnbd-fuzz-wrapper testcase - * If it's good, copy it to the test case directory: - * $ mv testcase fuzzing/testcase_dir/ - */ -static const char *write_testcase; - static void client (int s); static void server (int fd, int s); -static void nbdkit (int fd, int s); int main (int argc, char *argv[]) @@ -73,16 +55,7 @@ main (int argc, char *argv[]) pid_t pid; int sv[2], r, status; - if (argc == 3 && strcmp (argv[1], "--write") == 0) { - write_testcase = argv[2]; - - fd = open (argv[2], O_WRONLY|O_TRUNC|O_CREAT, 0644); - if (fd == -1) { - perror (argv[1]); - exit (EXIT_FAILURE); - } - } - else if (argc == 2) { + if (argc == 2) { /* Open the test case before we fork so we know the file exists. */ fd = open (argv[1], O_RDONLY); if (fd == -1) { @@ -133,10 +106,7 @@ main (int argc, char *argv[]) /* Child: NBD server. */ close (sv[0]); - if (!write_testcase) - server (fd, sv[1]); - else - nbdkit (fd, sv[1]); + server (fd, sv[1]); close (sv[1]); @@ -320,143 +290,3 @@ server (int fd, int sock) } } /* for (;;) */ } - -static void xwrite (int fd, const char *buf, size_t n); - -/* This is used for --write mode where we capture nbdkit output into a - * testcase file. - */ -static void -nbdkit (int fd, int sock) -{ - pid_t pid; - int rfd[2], wfd[2]; - struct pollfd pfds[2]; - char buf[512]; - ssize_t r; - bool parent_dead = false; - - if (pipe (rfd) == -1) { /* Will be our input, nbdkit's stdout */ - perror ("pipe"); - _exit (EXIT_FAILURE); - } - - if (pipe (wfd) == -1) { /* Will be our output, nbdkit's stdin */ - perror ("pipe"); - _exit (EXIT_FAILURE); - } - - /* Run nbdkit as another subprocess. */ - pid = fork (); - if (pid == -1) { - perror ("fork"); - _exit (EXIT_FAILURE); - } - - if (pid == 0) { /* Child - nbdkit */ - close (fd); - close (sock); - - close (rfd[0]); - dup2 (rfd[1], STDOUT_FILENO); - - close (wfd[1]); - dup2 (wfd[0], STDIN_FILENO); - - execlp ("nbdkit", - "nbdkit", "--exit-with-parent", "-s", - "sparse-random", - "--filter=cow", - "size=1M", - "runlength=8192", "percent=50", "random-content=true", - NULL); - perror ("execlp"); - _exit (EXIT_FAILURE); - } - - /* Here we shuffle the data: - * - * Our parent This process Our child - * (libnbd) (nbdkit) - * -------------> ------> -----wfd[1]----> - * sock - * <------------- <------ /<----rfd[0]----- - * | - * v - * writes from nbdkit tee'd to fd (testcase) - * - * We do everything blocking because that is easier and performance - * doesn't matter since we're only capturing a test case. - */ - close (rfd[1]); - close (wfd[0]); - - while (!parent_dead || rfd[0] >= 0) { - pfds[0].fd = parent_dead ? -1 : sock; - pfds[0].events = POLLIN; - pfds[0].revents = 0; - - pfds[1].fd = rfd[0]; - pfds[1].events = POLLIN; - pfds[1].revents = 0; - - if (poll (pfds, 2, -1) == -1) { - if (errno == EINTR) - continue; - perror ("poll"); - _exit (EXIT_FAILURE); - } - - if (!parent_dead && (pfds[0].revents & POLLIN) != 0) { - r = read (sock, buf, sizeof buf); - if (r == -1 && errno != EINTR) { - perror ("read (libnbd)"); - _exit (EXIT_FAILURE); - } - else if (r == 0) { - parent_dead = true; - continue; - } - else if (r > 0) - xwrite (wfd[1], buf, r); - } - - if (rfd[0] != -1 && (pfds[1].revents & POLLIN) != 0) { - r = read (rfd[0], buf, sizeof buf); - if (r == -1 && errno == EINTR) { - perror ("read (nbdkit)"); - _exit (EXIT_FAILURE); - } - else if (r == 0) { - close (rfd[0]); - rfd[0] = -1; - continue; - } - else if (r > 0) { - xwrite (fd, buf, r); - xwrite (sock, buf, r); - } - } - } - - if (close (fd) == -1) { - perror ("close"); - _exit (EXIT_FAILURE); - } -} - -static void -xwrite (int fd, const char *buf, size_t n) -{ - ssize_t r; - - while (n > 0) { - r = write (fd, buf, n); - if (r == -1) { - perror ("write"); - _exit (EXIT_FAILURE); - } - buf += r; - n -= r; - } -} -- 2.43.1
This adds a fuzzed data provider, the C equivalent of: https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/fuzzer/Fu... https://github.com/google/fuzzing/blob/master/docs/split-inputs.md#fuzzed-da... This is not very well documented except by reading the LLVM sources, but the general idea is that we define a format which is robust against the sort of input changes that fuzzers do, such as flipping bits, or inserting and removing bytes. This means, for example, that we cannot use any <length> + <data> fields (eg. for binary data) because a fuzzer would easily break this, either by flipping bits in the <length> field without "knowing" that it needs to adjust the <data>, or by inserting or removing bytes from <data> without "knowing" that it needs to update the <length>. Various other techniques are employed to improve robustness which are described in the code. Note the license is BSD because we want to use this in future in nbdkit. --- fuzzing/Makefile.am | 16 ++- fuzzing/fuzzed-data-provider.h | 223 +++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 3 deletions(-) diff --git a/fuzzing/Makefile.am b/fuzzing/Makefile.am index 450aaf1..1d878ee 100644 --- a/fuzzing/Makefile.am +++ b/fuzzing/Makefile.am @@ -29,10 +29,20 @@ if ENABLE_LIBFUZZER noinst_PROGRAMS += libnbd-libfuzzer-test endif -libnbd_fuzz_wrapper_SOURCES = libnbd-fuzz-wrapper.c -libnbd_fuzz_wrapper_CPPFLAGS = -I$(top_srcdir)/include +libnbd_fuzz_wrapper_SOURCES = \ + libnbd-fuzz-wrapper.c \ + fuzzer-data-provider.h \ + $(NULL) +libnbd_fuzz_wrapper_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/vector \ + $(NULL) libnbd_fuzz_wrapper_CFLAGS = $(WARNINGS_CFLAGS) -libnbd_fuzz_wrapper_LDADD = $(top_builddir)/lib/libnbd.la +libnbd_fuzz_wrapper_LDADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(top_builddir)/lib/libnbd.la \ + $(NULL) libnbd_libfuzzer_test_SOURCES = libnbd-libfuzzer-test.c libnbd_libfuzzer_test_CPPFLAGS = -I$(top_srcdir)/include diff --git a/fuzzing/fuzzed-data-provider.h b/fuzzing/fuzzed-data-provider.h new file mode 100644 index 0000000..8f836f6 --- /dev/null +++ b/fuzzing/fuzzed-data-provider.h @@ -0,0 +1,223 @@ +/* fuzzed-data-provider.h + * Copyright Red Hat + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of Red Hat nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* This is a C equivalent of the LLVM header: + * https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/fuzzer/Fu... + * See also: + * https://github.com/google/fuzzing/blob/master/docs/split-inputs.md#fuzzed-da... + */ + +#ifndef FUZZED_DATA_PROVIDER_H +#define FUZZED_DATA_PROVIDER_H + +#include <stdlib.h> +#include <stdbool.h> +#include <stdint.h> + +#include <minmax.h> +#include <vector.h> +#include <string-vector.h> +#include <const-string-vector.h> + +/* Before calling any other functions, your code must declare: + * + * byte_vector fuzzed_data; + * + * and 'fuzzed_data' must be initialized with the full data read from + * the fuzzer. + * + */ +DEFINE_VECTOR_TYPE (byte_vector, uint8_t); +extern byte_vector fuzzed_data; + +/* Test if there is any more fuzzer input to consume. You can use + * this to end the test if you're using a loop (because the other + * functions in this file will always return something even if there's + * no more input, which would cause an infinite loop). + * + * Implementation note: We actually consume bytes from the end of the + * fuzzer input first, as that is easier with our vector type. This + * should make no practical difference. + */ +static inline bool +more_fuzzed_data (void) +{ + return fuzzed_data.len > 0; +} + +/* Return exactly one byte from the fuzzed input. + * + * If there is no input, this returns zero. + */ +static inline uint8_t +fuzzed_data_consume_byte (void) +{ + uint8_t r = 0; + + if (fuzzed_data.len > 0) { + r = fuzzed_data.ptr[fuzzed_data.len-1]; + fuzzed_data.len--; + } + return r; +} + +/* Return exactly 'size' bytes into a preallocated buffer. The + * 'result' buffer must be at least 'size' bytes long. + * + * If there is insufficient input, this pads the output with zeroes. + */ +static inline void +fuzzed_data_consume_bytes (size_t size, void *result) +{ + uint8_t *r = result; + size_t n; + + n = MIN (size, fuzzed_data.len); + memcpy (r, &fuzzed_data.ptr[fuzzed_data.len - n], n); + memset (&r[n], 0, size-n); + + fuzzed_data.len -= n; +} + +/* Return exactly 'size' bytes, as a newly allocated vector. + * + * If there is insufficient input, this pads the output with zeroes. + */ +static inline byte_vector +fuzzed_data_consume_bytes_as_vector (size_t size) +{ + byte_vector result = empty_vector; + size_t n; + + /* We want to use reserve_exactly here so that ASAN can detect + * buffer overflow in the caller (and also because we are not + * expecting to need to expand this vector). + */ + if (byte_vector_reserve_exactly (&result, size) == -1) + abort (); + + fuzzed_data_consume_bytes (size, result.ptr); + return result; +} + +/* Return a variable sized buffer of data. The size of the data is + * controlled by the fuzzer itself, in a manner which is robust to + * normal fuzzer mutations. + * + * Implementation: We scan the input until we hit a single zero byte + * followed by a non-zero byte, and then we stop scanning and return + * the buffer. Two zero bytes in a row are treated as an "escape + * sequence", adding a zero byte to the output buffer and continuing + * scanning. If at any point during scanning we hit the end of the + * fuzzer input, we stop scanning and return the buffer. + * + * If there is no input, this returns a zero length buffer. + */ +static inline byte_vector +fuzzed_data_consume_buffer (size_t size) +{ + byte_vector result = empty_vector; + uint8_t b; + + while (more_fuzzed_data ()) { + b = fuzzed_data_consume_byte (); + if (b > 0) { + if (byte_vector_append (&result, b) == -1) + abort (); + } + else if (more_fuzzed_data ()) { + b = fuzzed_data_consume_byte (); + if (b > 0) { + /* End of the buffer marker = zero byte + non-zero byte. + * "Put back" the non-zero byte we just consumed. + */ + fuzzed_data.len++; + break; + } + else { + /* Escape sequence = zero byte + zero byte. + * Store a single zero byte and continue scanning. + */ + if (byte_vector_append (&result, 0) == -1) + abort (); + } + } + } + + return result; +} + +/* Return an unsigned integer between bounds [min, max] inclusive. + * + * If there is no more input, this returns 'min'. + */ +static inline uint64_t +fuzzed_data_consume_uint64_t (uint64_t min, uint64_t max) +{ + uint64_t range, result; + size_t offset_bits; + + if (min > max) + abort (); + + range = max - min; + result = 0; + offset_bits = 0; + while (offset_bits < 64 && (range >> offset) > 0) { + result <<= 8; + result |= fuzzed_data_consume_byte (); + } + + /* Make sure the result is in range, but avoid division by 0 if + * range + 1 would overflow. + */ + if (range < UINT64_MAX) + result = result % (range + 1); + + return min + result; +} + +/* Same as above, but returns an unsigned between [min..max] (inclusive). */ +static inline unsigned +fuzzed_data_consume_unsigned (unsigned min, unsigned max) +{ + return fuzzed_data_consume_uint64_t (min, max); +} + +/* Same as above, but returns an int between [0..max] (inclusive). */ +static inline int +fuzzed_data_consume_enum (int max) +{ + return fuzzed_data_consume_uint64_t (0, max); +} + +#endif /* FUZZED_DATA_PROVIDER_H */ -- 2.43.1
--- fuzzing/libnbd-fuzz-wrapper.c | 43 +++++++++++++++++++++++++++------ fuzzing/libnbd-libfuzzer-test.c | 4 +++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/fuzzing/libnbd-fuzz-wrapper.c b/fuzzing/libnbd-fuzz-wrapper.c index 338adc0..fbff095 100644 --- a/fuzzing/libnbd-fuzz-wrapper.c +++ b/fuzzing/libnbd-fuzz-wrapper.c @@ -17,10 +17,25 @@ */ /* This is a wrapper allowing libnbd to be tested using common fuzzers - * such as afl. It takes the fuzzer test case as a filename on the - * command line. This is fed to the libnbd socket. Any output to the - * socket from libnbd is sent to /dev/null. This is basically the - * same way we fuzz nbdkit, but in reverse (see nbdkit.git/fuzzing). + * such as AFL++. It takes the fuzzer test case as a filename on the + * command line. + * + * It uses fuzzed-data-provider.h to parse the input allowing a choice + * of APIs to be called in any order under control of the fuzzer. The + * test cases therefore do not correspond very closely to raw NBD + * protocol. + * + * The fuzzer input is parsed as a series of commands, directing this + * program to execute different libnbd APIs. When the input is + * exhausted we stop the test. Each command is structured as: + * + * <enum of API to call> + * <offset> + * <flags> + * <data buffer> (includes implicit length) + * + * The data buffer field is only used by nbd_aio_pwrite. For other + * APIs only the length is used and the content is ignored. */ #include <config.h> @@ -146,10 +161,20 @@ extent64_callback (void *user_data, return 0; } +enum call_type { + PREAD, + PWRITE, + FLUSH, + TRIM, + ZERO, + CACHE, + PREAD_STRUCTURED, + BLOCK_STATUS, + BLOCK_STATUS_64, + CALL_TYPE_MAX_PLUS_1 /* must be last entry */ +}; + /* This is the client (parent process) running libnbd. */ -static char buf[512]; -static char prbuf[65536]; - static void client (int sock) { @@ -180,6 +205,10 @@ client (int sock) length = nbd_get_size (nbd); + /* Main loop: Consume fuzzer data to decide which calls we will make. */ + while (more_fuzzed_data + + /* Test common asynchronous I/O calls. */ nbd_aio_pread (nbd, buf, sizeof buf, 0, NBD_NULL_COMPLETION, 0); nbd_aio_pwrite (nbd, buf, sizeof buf, 0, NBD_NULL_COMPLETION, 0); diff --git a/fuzzing/libnbd-libfuzzer-test.c b/fuzzing/libnbd-libfuzzer-test.c index 1721b74..cf6c1cf 100644 --- a/fuzzing/libnbd-libfuzzer-test.c +++ b/fuzzing/libnbd-libfuzzer-test.c @@ -22,6 +22,10 @@ * * - This case is mostly unmaintained. The maintainers use AFL++ for * fuzzing (see libnbd-fuzz-wrapper.c). + * + * - This test needs to be updated to use the new method of reading + * input via the fuzzed data provider (see again + * libnbd-fuzz-wrapper.c). */ #include <config.h> -- 2.43.1
participants (1)
-
Richard W.M. Jones