Re: [PATCH] fuzz: pass failures from child process into libfuzzer engine

[Konstantin Khlebnikov]

 

 

06.12.2021, 19:35, "Alexander Bulekov" <alxndr@bu.edu>:

On 211205 1917, Konstantin Khlebnikov wrote:

 Fuzzer is supposed to stop when first bug is found and report failure.
 Present fuzzers fork new child at each iteration to isolate side-effects.
 But child's exit code is ignored, i.e. libfuzzer does not see any crashes.
 
 Right now virtio-net fuzzer instantly falls on assert in iov_copy and
 dumps crash-*, but fuzzing continues and ends successfully if global
 timeout is set.
 
 Let's put required logic into helper function "fork_fuzzer_and_wait".
 

Hi Konstantin,
Can you provide more details about them problem this is meant to solve?
Currently, the fuzzer would just output a "crash-" file and continue
fuzzing. So the crash isn't lost - it can still be reproduced later.
This means the fuzzer can progress faster (no need to restart the whole
process each time there is a crash).

However, this is of course, not the default libfuzzer behavior. That's
why I wonder whether you encountered some issue that depended on
libfuzzer exiting immediately. We have had some problems on OSS-Fuzz,
with incomplete coverage reports, and I wonder if this could be related.

For the example you gave, OSS-Fuzz picked up on the crash, so even
though we don't comform to the default libfuzzer behavior, the crashes
are still detected.
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=23985&q=iov_copy&can=2

Oh well. So, this is known "feature". That was unexpected. =)

Recent libfuzzer has options for that behaviour: "-fork=1 -ignore_crashes=1".

 

I'm trying to fuzz various virtio devices and was really surprised that present

fuzzing targes still find crashes in seconds.

I thought they might be missed due to unhandled exit status.

 

It seems fuzzing targets like "virtio-net" in present state wastes resources.

oss-fuzz could instead focus on not yet broken targets.

 

Or "abort/assert' in device emulation code should be treated as "success"?

In some sense that's true, we cannot prevent suicide behaviour in vm.

Real hardware dies easily after shooting randomly into ports/io ranges.

Small question below.

 Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
 ---
  tests/qtest/fuzz/fork_fuzz.c | 26 ++++++++++++++++++++++++++
  tests/qtest/fuzz/fork_fuzz.h | 1 +
  tests/qtest/fuzz/generic_fuzz.c | 3 +--
  tests/qtest/fuzz/i440fx_fuzz.c | 3 +--
  tests/qtest/fuzz/virtio_blk_fuzz.c | 6 ++----
  tests/qtest/fuzz/virtio_net_fuzz.c | 6 ++----
  tests/qtest/fuzz/virtio_scsi_fuzz.c | 6 ++----
  7 files changed, 35 insertions(+), 16 deletions(-)
 
 diff --git a/tests/qtest/fuzz/fork_fuzz.c b/tests/qtest/fuzz/fork_fuzz.c
 index 6ffb2a7937..6e3a3867bf 100644
 --- a/tests/qtest/fuzz/fork_fuzz.c
 +++ b/tests/qtest/fuzz/fork_fuzz.c
 @@ -38,4 +38,30 @@ void counter_shm_init(void)
      free(copy);
  }
  
 +/* Returns true in child process */
 +bool fork_fuzzer_and_wait(void)
 +{
 + pid_t pid;
 + int wstatus;
 +
 + pid = fork();
 + if (pid < 0) {
 + perror("fork");
 + abort();
 + }
 +
 + if (pid == 0) {
 + return true;
 + }
  
 + if (waitpid(pid, &wstatus, 0) < 0) {
 + perror("waitpid");
 + abort();
 + }
 +
 + if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) {
 + abort();
 + }

Maybe instead of these aborts, we return "true" so the fork-server tries
to run the input, itself and (hopefully) crashes. That way we would have
an accurate stack trace, instead of abort, which is probably important
for the OSS-Fuzz crash-bucketing.

Stack trace from child process should have same accuracy.

I don't see how they could be different.

 

I suppose OSS-Fuzz infrastructure is ready to handle multiple stacktraces,

e.g. stacktraces from several threads should be a common result.

 

Thanks
-Alex
 

 +
 + return false;
 +}
 diff --git a/tests/qtest/fuzz/fork_fuzz.h b/tests/qtest/fuzz/fork_fuzz.h
 index 9ecb8b58ef..792e922731 100644
 --- a/tests/qtest/fuzz/fork_fuzz.h
 +++ b/tests/qtest/fuzz/fork_fuzz.h
 @@ -18,6 +18,7 @@ extern uint8_t __FUZZ_COUNTERS_START;
  extern uint8_t __FUZZ_COUNTERS_END;
  
  void counter_shm_init(void);
 +bool fork_fuzzer_and_wait(void);
  
  #endif
  
 diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
 index dd7e25851c..f0e25b39ea 100644
 --- a/tests/qtest/fuzz/generic_fuzz.c
 +++ b/tests/qtest/fuzz/generic_fuzz.c
 @@ -667,7 +667,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
      size_t cmd_len;
      uint8_t op;
  
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          struct sigaction sact;
          struct itimerval timer;
          sigset_t set;
 @@ -723,7 +723,6 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
          _Exit(0);
      } else {
          flush_events(s);
 - wait(0);
      }
  }
  
 diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
 index 86796bff2b..0b927f4b3a 100644
 --- a/tests/qtest/fuzz/i440fx_fuzz.c
 +++ b/tests/qtest/fuzz/i440fx_fuzz.c
 @@ -147,12 +147,11 @@ static void i440fx_fuzz_qos(QTestState *s,
  
  static void i440fx_fuzz_qos_fork(QTestState *s,
          const unsigned char *Data, size_t Size) {
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          i440fx_fuzz_qos(s, Data, Size);
          _Exit(0);
      } else {
          flush_events(s);
 - wait(NULL);
      }
  }
  
 diff --git a/tests/qtest/fuzz/virtio_blk_fuzz.c b/tests/qtest/fuzz/virtio_blk_fuzz.c
 index 623a756fd4..9532dc1fa7 100644
 --- a/tests/qtest/fuzz/virtio_blk_fuzz.c
 +++ b/tests/qtest/fuzz/virtio_blk_fuzz.c
 @@ -136,13 +136,12 @@ static void virtio_blk_fork_fuzz(QTestState *s,
      if (!queues) {
          queues = qvirtio_blk_init(blk->vdev, 0);
      }
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          virtio_blk_fuzz(s, queues, Data, Size);
          flush_events(s);
          _Exit(0);
      } else {
          flush_events(s);
 - wait(NULL);
      }
  }
  
 @@ -152,7 +151,7 @@ static void virtio_blk_with_flag_fuzz(QTestState *s,
      QVirtioBlk *blk = fuzz_qos_obj;
      static QVirtioBlkQueues *queues;
  
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          if (Size >= sizeof(uint64_t)) {
              queues = qvirtio_blk_init(blk->vdev, *(uint64_t *)Data);
              virtio_blk_fuzz(s, queues,
 @@ -162,7 +161,6 @@ static void virtio_blk_with_flag_fuzz(QTestState *s,
          _Exit(0);
      } else {
          flush_events(s);
 - wait(NULL);
      }
  }
  
 diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
 index 0e873ab8e2..6b492ef9e7 100644
 --- a/tests/qtest/fuzz/virtio_net_fuzz.c
 +++ b/tests/qtest/fuzz/virtio_net_fuzz.c
 @@ -118,26 +118,24 @@ static void virtio_net_fuzz_multi(QTestState *s,
  static void virtio_net_fork_fuzz(QTestState *s,
          const unsigned char *Data, size_t Size)
  {
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          virtio_net_fuzz_multi(s, Data, Size, false);
          flush_events(s);
          _Exit(0);
      } else {
          flush_events(s);
 - wait(NULL);
      }
  }
  
  static void virtio_net_fork_fuzz_check_used(QTestState *s,
          const unsigned char *Data, size_t Size)
  {
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          virtio_net_fuzz_multi(s, Data, Size, true);
          flush_events(s);
          _Exit(0);
      } else {
          flush_events(s);
 - wait(NULL);
      }
  }
  
 diff --git a/tests/qtest/fuzz/virtio_scsi_fuzz.c b/tests/qtest/fuzz/virtio_scsi_fuzz.c
 index 6ff6fabe4a..c7eaf3242b 100644
 --- a/tests/qtest/fuzz/virtio_scsi_fuzz.c
 +++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
 @@ -140,13 +140,12 @@ static void virtio_scsi_fork_fuzz(QTestState *s,
      if (!queues) {
          queues = qvirtio_scsi_init(scsi->vdev, 0);
      }
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          virtio_scsi_fuzz(s, queues, Data, Size);
          flush_events(s);
          _Exit(0);
      } else {
          flush_events(s);
 - wait(NULL);
      }
  }
  
 @@ -156,7 +155,7 @@ static void virtio_scsi_with_flag_fuzz(QTestState *s,
      QVirtioSCSI *scsi = fuzz_qos_obj;
      static QVirtioSCSIQueues *queues;
  
 - if (fork() == 0) {
 + if (fork_fuzzer_and_wait()) {
          if (Size >= sizeof(uint64_t)) {
              queues = qvirtio_scsi_init(scsi->vdev, *(uint64_t *)Data);
              virtio_scsi_fuzz(s, queues,
 @@ -166,7 +165,6 @@ static void virtio_scsi_with_flag_fuzz(QTestState *s,
          _Exit(0);
      } else {
          flush_events(s);
 - wait(NULL);
      }
  }