Richard W.M. Jones
2019-Nov-04 21:31 UTC
[Libguestfs] [PATCH nbdkit v2 0/2] Implement fuzzing using Clang's libFuzzer.
v1 was here: https://www.redhat.com/archives/libguestfs/2019-November/msg00003.html This version depends on: https://www.redhat.com/archives/libguestfs/2019-November/msg00004.html and this series: https://www.redhat.com/archives/libguestfs/2019-November/msg00009.html The delta has been reduced slightly because of changes made possible by cleaning up and fixing the quit path in nbdkit. It's still a fairly ugly and invasive patch however, so suggestions sought about how to make it better. Rich.
Richard W.M. Jones
2019-Nov-04 21:31 UTC
[Libguestfs] [PATCH nbdkit v2 1/2] server: Close the quit pipe-to-self when exiting.
--- server/internal.h | 1 + server/main.c | 7 ++++--- server/quit.c | 7 +++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/server/internal.h b/server/internal.h index 4638727..7069df0 100644 --- a/server/internal.h +++ b/server/internal.h @@ -127,6 +127,7 @@ extern struct backend *backend; extern volatile int quit; extern int quit_fd; extern void set_up_quit_pipe (void); +extern void close_quit_pipe (void); extern void handle_quit (int sig); extern void set_quit (void); diff --git a/server/main.c b/server/main.c index 8ff8e79..9ccba4a 100644 --- a/server/main.c +++ b/server/main.c @@ -682,6 +682,9 @@ main (int argc, char *argv[]) /* Select the correct thread model based on config. */ lock_init_thread_model (); + set_up_quit_pipe (); + set_up_signals (); + start_serving (); backend->free (backend); @@ -701,6 +704,7 @@ main (int argc, char *argv[]) } crypto_free (); + close_quit_pipe (); exit (EXIT_SUCCESS); } @@ -865,9 +869,6 @@ start_serving (void) exit (EXIT_FAILURE); } - set_up_quit_pipe (); - set_up_signals (); - /* Socket activation -- we are handling connections on pre-opened * file descriptors [FIRST_SOCKET_ACTIVATION_FD .. * FIRST_SOCKET_ACTIVATION_FD+nr_socks-1]. diff --git a/server/quit.c b/server/quit.c index ffd3fec..5a916b2 100644 --- a/server/quit.c +++ b/server/quit.c @@ -80,6 +80,13 @@ set_up_quit_pipe (void) write_quit_fd = fds[1]; } +void +close_quit_pipe (void) +{ + close (quit_fd); + close (write_quit_fd); +} + void handle_quit (int sig) { -- 2.23.0
Richard W.M. Jones
2019-Nov-04 21:31 UTC
[Libguestfs] [PATCH nbdkit v2 2/2] fuzzing: Implement fuzzing using Clang’s libFuzzer.
This works by building a special version of nbdkit as a kind of library, linked against libFuzzer (note that libFuzzer provides the main() function). The test entry point forks a subprocess which feeds input to nbdkit, working in a similar way to libnbd’s fuzzing/libnbd-libfuzzer-test.c. --- .gitignore | 1 + Makefile.am | 9 +- TODO | 3 +- configure.ac | 12 +++ fuzzing/README | 43 +++++++++- server/Makefile.am | 13 +++ server/fuzzer.c | 208 +++++++++++++++++++++++++++++++++++++++++++++ server/internal.h | 5 +- server/main.c | 24 +++++- 9 files changed, 308 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 22a20ef..b6cfd73 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,7 @@ Makefile.in /docs/lang-plugin-links.pod /docs/plugin-links.pod /fuzzing/sync_dir/ +/fuzzing/testcase_dir/[0-f]* /html/*.html /include/nbdkit-version.h /INSTALL diff --git a/Makefile.am b/Makefile.am index 707751f..3daf9fb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -47,6 +47,7 @@ EXTRA_DIST = \ CLEANFILES += html/*.html +if !ENABLE_LIBFUZZER # NB: This is not the real nbdkit binary. It's a wrapper that allows # you to run nbdkit from the build directory before it is installed. noinst_PROGRAMS = nbdkit @@ -63,6 +64,7 @@ nbdkit_LDADD = \ $(top_builddir)/common/utils/libutils.la \ $(NULL) nbdkit_DEPENDENCIES = config.status +endif !ENABLE_LIBFUZZER SUBDIRS = \ bash \ @@ -73,9 +75,14 @@ SUBDIRS = \ common/include \ common/protocol \ common/utils \ - server \ $(NULL) +if ENABLE_LIBFUZZER +SUBDIRS += common/sparse +endif + +SUBDIRS += server + if HAVE_PLUGINS SUBDIRS += \ common/bitmap \ diff --git a/TODO b/TODO index 03fa656..fa7ec5f 100644 --- a/TODO +++ b/TODO @@ -113,8 +113,7 @@ General ideas for improvements good enough? Also allowing multiple instances of nbdkit to be loaded in the same process is probably impossible. -* Examine other fuzzers: oss-fuzz, libfuzzer, fuzzit, and whatever it - is that syzkaller uses. +* Examine other fuzzers: https://gitlab.com/akihe/radamsa * Analyze code with Coverity. diff --git a/configure.ac b/configure.ac index cb0351e..658391d 100644 --- a/configure.ac +++ b/configure.ac @@ -391,6 +391,18 @@ AS_IF([test "x$enable_valgrind" = "xyes"],[ ]) ]) +dnl Build the special libFuzzer version of nbdkit. DO NOT USE THIS for +dnl normal builds. See fuzzing/README. +AC_ARG_ENABLE([libfuzzer], + [AS_HELP_STRING([--enable-libfuzzer], + [build libFuzzer test binary (developers only)])], + [enable_libfuzzer=yes], + [enable_libfuzzer=no]) +AS_IF([test "x$enable_libfuzzer" = "xyes"],[ + AC_DEFINE([ENABLE_LIBFUZZER],[1],[Enable special libFuzzer binary]) +]) +AM_CONDITIONAL([ENABLE_LIBFUZZER],[test "x$enable_libfuzzer" = "xyes"]) + dnl Bash completion. PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >= 2.0], [ bash_completion=yes diff --git a/fuzzing/README b/fuzzing/README index 9fcce89..fe5a283 100644 --- a/fuzzing/README +++ b/fuzzing/README @@ -1,5 +1,7 @@ +To report security bugs, see ‘SECURITY’ in the top source directory. + Fuzzing nbdkit using the American Fuzzy Lop (afl) fuzzer --------------------------------------------------------- +======================================================= We can fuzz nbdkit using the -s (read from stdin) option, and feeding in a file of "network" input. The stdout from nbdkit (ie. data that @@ -57,8 +59,6 @@ This will create an HTML test coverage report in Notes ----- -To report security bugs, see ‘SECURITY’ in the top source directory. - We only have testcases for the newstyle fixed protocol so far, but if people report that they are exposing the oldstyle protocol to the internet / untrusted clients then we could extend the testing for @@ -68,3 +68,40 @@ We have only fuzzed the memory plugin, which is convenient because it stores everything in memory and throws it away when nbdkit exits. Since plugins can have bugs as well as the core server, it may be productive to test other plugins and filters. + +Fuzzing nbdkit using Clang + libFuzzer +=====================================+ +Recompile nbdkit with libFuzzer enabled and build the libFuzzer test +binary: + + ./configure \ + CC=clang \ + CFLAGS="-g -O1" \ + --enable-libfuzzer \ + --disable-perl + make clean + make CFLAGS="-g -O1 -fsanitize=fuzzer,address" + +(The awkward additional CFLAGS on the make command line are necessary +because ./configure attempts to test that the compiler works, but this +test fails when -fsanitize=fuzzer is used as that option adds an extra +main() definition.) + +",address" enables the Clang Address Sanitizer, and can be omitted for +faster fuzzing. + +This builds a specialized version of nbdkit which is the fuzzer test +program (testing only nbdkit-memory-plugin unless you modify the +source). You can run it like this on the input corpus: + + ./server/nbdkit fuzzing/testcase_dir + +New test inputs are written to fuzzing/testcase_dir and will be used +on subsequent runs. If this is undesirable then delete +fuzzing/testcase_dir/[0-f]* before the run. + +There are various extra command line options supported by libFuzzer. +For more details see: + + https://llvm.org/docs/LibFuzzer.html diff --git a/server/Makefile.am b/server/Makefile.am index c846fc7..cc148ac 100644 --- a/server/Makefile.am +++ b/server/Makefile.am @@ -102,6 +102,19 @@ if USE_LINKER_SCRIPT_FOR_SERVER nbdkit_LDFLAGS += -Wl,--version-script=$(srcdir)/nbdkit.syms endif +if ENABLE_LIBFUZZER +nbdkit_SOURCES += \ + fuzzer.c \ + $(top_srcdir)/plugins/memory/memory.c \ + $(NULL) +nbdkit_CPPFLAGS += \ + -I$(top_srcdir)/common/sparse \ + $(NULL) +nbdkit_LDFLAGS += \ + $(top_builddir)/common/sparse/libsparse.la \ + $(NULL) +endif + # synopsis.c is generated from docs/synopsis.txt where it is also # used to generate the man page. It is included in main.c. diff --git a/server/fuzzer.c b/server/fuzzer.c new file mode 100644 index 0000000..f1c525f --- /dev/null +++ b/server/fuzzer.c @@ -0,0 +1,208 @@ +/* nbdkit + * Copyright (C) 2013-2019 Red Hat Inc. + * + * 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. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <unistd.h> +#include <poll.h> +#include <errno.h> +#include <sys/types.h> +#include <sys/socket.h> +#include <sys/wait.h> + +#include "internal.h" + +#ifdef ENABLE_LIBFUZZER + +/* When we're compiled with --enable-libfuzzer, the normal main() + * function is renamed to fuzzer_main. We call it with particular + * parameters to make it use the memory plugin. + */ +extern int fuzzer_main (int argc, char *argv[]); + +/* When compiled with --enable-libfuzzer we are linked directly with + * the memory plugin. We provide a specialized open_plugin_so called + * from fuzzer_main which registers the already loaded plugin. + */ +struct backend * +open_plugin_so (size_t i, const char *name, int short_name) +{ + extern struct nbdkit_plugin *plugin_init (void); + + return plugin_register (i, name, NULL, plugin_init); +} + +static void server (int sock); +static void client (const uint8_t *data, size_t size, int sock); + +/* This is the entry point called by libFuzzer. */ +int +LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) +{ + pid_t pid; + int sv[2], r, status; + + /* Create a connected socket. */ + if (socketpair (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, sv) == -1) { + perror ("socketpair"); + exit (EXIT_FAILURE); + } + + /* Fork: The parent will be the nbdkit process (server). The child + * will be the phony NBD client. + */ + pid = fork (); + if (pid == -1) { + perror ("fork"); + exit (EXIT_FAILURE); + } + + if (pid > 0) { + /* Parent: nbdkit server. */ + close (sv[1]); + + server (sv[0]); + + close (sv[0]); + + again: + r = wait (&status); + if (r == -1) { + if (errno == EINTR) + goto again; + perror ("wait"); + exit (EXIT_FAILURE); + } + if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) + fprintf (stderr, "bad exit status %d\n", status); + + return 0; + } + + /* Child: phony NBD client. */ + close (sv[0]); + + client (data, size, sv[1]); + + close (sv[1]); + + _exit (EXIT_SUCCESS); +} + +static void +server (int sock) +{ + char *argv[] = { "nbdkit", + "-s", /* take input from stdin/stdout */ + "--log=null", /* discard error messages */ + "./plugins/memory/.libs/nbdkit-memory-plugin.so", "1M", + NULL }; + const int argc = sizeof argv / sizeof argv[0] - 1; + int saved_stdin, saved_stdout; + + /* Make the socket appear as stdin and stdout of the process, saving + * the existing stdin/stdout. + */ + saved_stdin = dup (0); + saved_stdout = dup (1); + dup2 (sock, 0); + dup2 (sock, 1); + + /* Call nbdkit's normal main() function. */ + fuzzer_main (argc, argv); + + /* Restore stdin/stdout. */ + dup2 (saved_stdin, 0); + dup2 (saved_stdout, 1); + close (saved_stdin); + close (saved_stdout); +} + +static void +client (const uint8_t *data, size_t size, int sock) +{ + struct pollfd pfds[1]; + char rbuf[512]; + ssize_t r; + + if (size == 0) + shutdown (sock, SHUT_WR); + + for (;;) { + pfds[0].fd = sock; + pfds[0].events = POLLIN; + if (size > 0) pfds[0].events |= POLLOUT; + pfds[0].revents = 0; + + if (poll (pfds, 1, -1) == -1) { + if (errno == EINTR) + continue; + perror ("poll"); + /* This is not an error. */ + return; + } + + /* We can read from the server socket. Just throw away anything sent. */ + if ((pfds[0].revents & POLLIN) != 0) { + r = read (sock, rbuf, sizeof rbuf); + if (r == -1 && errno != EINTR) { + //perror ("read"); + return; + } + else if (r == 0) /* end of input from the server */ + return; + } + + /* We can write to the server socket. */ + if ((pfds[0].revents & POLLOUT) != 0) { + if (size > 0) { + r = write (sock, data, size); + if (r == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { + perror ("write"); + return; + } + else if (r > 0) { + data += r; + size -= r; + + if (size == 0) + shutdown (sock, SHUT_WR); + } + } + } + } /* for (;;) */ +} + +#endif /* ENABLE_LIBFUZZER */ diff --git a/server/internal.h b/server/internal.h index 7069df0..bb01f46 100644 --- a/server/internal.h +++ b/server/internal.h @@ -68,6 +68,8 @@ # define DO_DLCLOSE !RUNNING_ON_VALGRIND #elif defined(__SANITIZE_ADDRESS__) # define DO_DLCLOSE 0 +#elif defined(ENABLE_LIBFUZZER) +# define DO_DLCLOSE 0 #else # define DO_DLCLOSE 1 #endif @@ -122,6 +124,7 @@ extern bool verbose; extern struct backend *backend; #define for_each_backend(b) for (b = backend; b != NULL; b = b->next) +extern struct backend *open_plugin_so (size_t i, const char *filename, int short_name); /* quit.c */ extern volatile int quit; @@ -448,7 +451,7 @@ extern int backend_cache (struct backend *b, struct connection *conn, /* plugins.c */ extern struct backend *plugin_register (size_t index, const char *filename, void *dl, struct nbdkit_plugin *(*plugin_init) (void)) - __attribute__((__nonnull__ (2, 3, 4))); + __attribute__((__nonnull__ (2, /* 3,*/ 4))); /* filters.c */ extern struct backend *filter_register (struct backend *next, size_t index, diff --git a/server/main.c b/server/main.c index 9ccba4a..4e836dc 100644 --- a/server/main.c +++ b/server/main.c @@ -60,8 +60,11 @@ #include "options.h" #include "exit-with-parent.h" +#ifdef ENABLE_LIBFUZZER +#define main fuzzer_main +#endif + static char *make_random_fifo (void); -static struct backend *open_plugin_so (size_t i, const char *filename, int short_name); static struct backend *open_filter_so (struct backend *next, size_t i, const char *filename, int short_name); static void start_serving (void); static void write_pidfile (void); @@ -168,7 +171,15 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } +#if !ENABLE_LIBFUZZER threadlocal_init (); +#else + static bool main_called = false; + if (!main_called) { + threadlocal_init (); + main_called = true; + } +#endif /* The default setting for TLS depends on whether we were * compiled with GnuTLS. @@ -683,7 +694,9 @@ main (int argc, char *argv[]) lock_init_thread_model (); set_up_quit_pipe (); +#if !ENABLE_LIBFUZZER set_up_signals (); +#endif start_serving (); @@ -706,7 +719,10 @@ main (int argc, char *argv[]) crypto_free (); close_quit_pipe (); - exit (EXIT_SUCCESS); + /* Note: Don't exit here, otherwise this won't work when compiled + * for libFuzzer. + */ + return EXIT_SUCCESS; } /* Implementation of '-U -' */ @@ -741,7 +757,8 @@ make_random_fifo (void) return sock; } -static struct backend * +#ifndef ENABLE_LIBFUZZER +struct backend * open_plugin_so (size_t i, const char *name, int short_name) { struct backend *ret; @@ -792,6 +809,7 @@ open_plugin_so (size_t i, const char *name, int short_name) return ret; } +#endif /* !ENABLE_LIBFUZZER */ static struct backend * open_filter_so (struct backend *next, size_t i, -- 2.23.0
Richard W.M. Jones
2019-Nov-04 21:39 UTC
Re: [Libguestfs] [PATCH nbdkit v2 0/2] Implement fuzzing using Clang's libFuzzer.
On Mon, Nov 04, 2019 at 09:31:09PM +0000, Richard W.M. Jones wrote:> v1 was here: > https://www.redhat.com/archives/libguestfs/2019-November/msg00003.html > > This version depends on: > https://www.redhat.com/archives/libguestfs/2019-November/msg00004.html > and this series: > https://www.redhat.com/archives/libguestfs/2019-November/msg00009.html > > The delta has been reduced slightly because of changes made possible > by cleaning up and fixing the quit path in nbdkit. It's still a > fairly ugly and invasive patch however, so suggestions sought about > how to make it better.I forgot to add a few points: * It still leaks memory slowly. I ran the v1 version overnight and it leaked about 1 GB. Be good to find out eventually what is leaking. * This patch might be further simplified if there was a configure option to link a particular plugin into nbdkit statically, eg: ./configure --enable-static-plugin=memory which is sort of a useful feature on its own. * I think the --log=null patch is a reasonable feature on its own. * We didn't find any crashes yet. However enabling ASAN finds problems in both libnbd & nbdkit which I didn't investigate yet (they are difficult to reproduce). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Seemingly Similar Threads
- [PATCH nbdkit 0/2] Implement fuzzing using Clang's libFuzzer.
- [PATCH nbdkit 1/2] server: Call .get_ready before redirecting stdin/stdout to /dev/null.
- Using Libfuzzer on a library - linking the library to the fuzz target
- Using Libfuzzer on a library - linking the library to the fuzz target
- Re: Plans for nbdkit 1.18 release?