[PATCH 0/4] fuzz: misc changes for oss-fuzz compatability

[PATCH 0/4] fuzz: misc changes for oss-fuzz compatability

[Alexander Bulekov]

Hello,
With these patches, the fuzzer passes the oss-fuzz build checks.
There are also some miscelanous improvement to the fuzzer, in general:
 * If building for oss-fuzz, check executable_dir/pc-bios for
   the bios images
 * Fix a typo in the i440fx-qtest-reboot argument which resulted in an
   invalid argument to qemu_main
 * Add an alternate name to resolve libfuzzer's internal fuzzer::TPC
   object at link-time
 * For all fork-based fuzzers, run the main-loop in the parent, to
   prevent the clock from running far-ahead of the previous main-loop.
-Alex

Alexander Bulekov (4):
  fuzz: add datadir for oss-fuzz compatability
  fuzz: fix typo in i440fx-qtest-reboot arguments
  fuzz: add mangled object name to linker script
  fuzz: run the main-loop in fork-server process

 include/sysemu/sysemu.h             |  2 ++
 softmmu/vl.c                        |  2 +-
 tests/qtest/fuzz/fork_fuzz.ld       |  5 +++++
 tests/qtest/fuzz/fuzz.c             | 15 +++++++++++++++
 tests/qtest/fuzz/i440fx_fuzz.c      |  3 ++-
 tests/qtest/fuzz/virtio_net_fuzz.c  |  2 ++
 tests/qtest/fuzz/virtio_scsi_fuzz.c |  2 ++
 7 files changed, 29 insertions(+), 2 deletions(-)

-- 
2.26.2

[PATCH 1/4] fuzz: add datadir for oss-fuzz compatability

[Alexander Bulekov]

This allows us to keep pc-bios in executable_dir/pc-bios, rather than
executable_dir/../pc-bios, which is incompatible with oss-fuzz' file
structure.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 include/sysemu/sysemu.h |  2 ++
 softmmu/vl.c            |  2 +-
 tests/qtest/fuzz/fuzz.c | 15 +++++++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ef81302e1a..cc96b66fc9 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -15,6 +15,8 @@ extern const char *qemu_name;
 extern QemuUUID qemu_uuid;
 extern bool qemu_uuid_set;
 
+void qemu_add_data_dir(const char *path);
+
 void qemu_add_exit_notifier(Notifier *notify);
 void qemu_remove_exit_notifier(Notifier *notify);
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index afd2615fb3..c71485a965 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1993,7 +1993,7 @@ char *qemu_find_file(int type, const char *name)
     return NULL;
 }
 
-static void qemu_add_data_dir(const char *path)
+void qemu_add_data_dir(const char *path)
 {
     int i;
 
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index f5c923852e..33365c3782 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -137,6 +137,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
 {
 
     char *target_name;
+    char *dir;
 
     /* Initialize qgraph and modules */
     qos_graph_init();
@@ -147,6 +148,20 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
     target_name = strstr(**argv, "-target-");
     if (target_name) {        /* The binary name specifies the target */
         target_name += strlen("-target-");
+        /*
+         * With oss-fuzz, the executable is kept in the root of a directory (we
+         * cannot assume the path). All data (including bios binaries) must be
+         * in the same dir, or a subdir. Thus, we cannot place the pc-bios so
+         * that it would be in exec_dir/../pc-bios.
+         * As a workaround, oss-fuzz allows us to use argv[0] to get the
+         * location of the executable. Using this we add exec_dir/pc-bios to
+         * the datadirs.
+         */
+        dir = g_build_filename(g_path_get_dirname(**argv), "pc-bios", NULL);
+        if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
+            qemu_add_data_dir(dir);
+        }
+        g_free(dir);
     } else if (*argc > 1) {  /* The target is specified as an argument */
         target_name = (*argv)[1];
         if (!strstr(target_name, "--fuzz-target=")) {
-- 
2.26.2

[PATCH 2/4] fuzz: fix typo in i440fx-qtest-reboot arguments

[Alexander Bulekov]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/i440fx_fuzz.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
index ab5f112584..90e75ffaea 100644
--- a/tests/qtest/fuzz/i440fx_fuzz.c
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -143,7 +143,7 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
 }
 
 static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
-                                       "-m 0 -display none";
+                                       " -m 0 -display none";
 static const char *i440fx_argv(FuzzTarget *t)
 {
     return i440fx_qtest_argv;
-- 
2.26.2

[PATCH 3/4] fuzz: add mangled object name to linker script

[Alexander Bulekov]

Previously, we relied on "FuzzerTracePC*(.bss*)" to place libfuzzer's
fuzzer::TPC object into our contiguous shared-memory region. This does
not work for some libfuzzer builds, so this addition identifies the
region by its mangled name: *(.bss._ZN6fuzzer3TPCE);

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/fork_fuzz.ld | 5 +++++
 1 file changed, 5 insertions(+)

This isn't ideal, but I looked at the libfuzzer builds packaged for
debian, for versions 6, 7, 8, 9, 10 and 11 and this (mangled) object
name appears consistently in the symbol tables.

diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld
index e086bba873..bfb667ed06 100644
--- a/tests/qtest/fuzz/fork_fuzz.ld
+++ b/tests/qtest/fuzz/fork_fuzz.ld
@@ -28,6 +28,11 @@ SECTIONS
 
       /* Internal Libfuzzer TracePC object which contains the ValueProfileMap */
       FuzzerTracePC*(.bss*);
+      /*
+       * In case the above line fails, explicitly specify the (mangled) name of
+       * the object we care about
+       */
+       *(.bss._ZN6fuzzer3TPCE);
   }
   .data.fuzz_end : ALIGN(4K)
   {
-- 
2.26.2

[PATCH 4/4] fuzz: run the main-loop in fork-server process

[Alexander Bulekov]

Without this, the time since the last main-loop keeps increasing, as the
fuzzer runs. The forked children need to handle all the "past-due"
timers, slowing them down, over time. With this change, the
parent/fork-server process runs the main-loop, while waiting on the
child, ensuring that the timer events do not pile up, over time.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/i440fx_fuzz.c      | 1 +
 tests/qtest/fuzz/virtio_net_fuzz.c  | 2 ++
 tests/qtest/fuzz/virtio_scsi_fuzz.c | 2 ++
 3 files changed, 5 insertions(+)

I'm working on another series to abstract away the details of resetting
qemu state between runs from the individual targets. That should relieve
us from needing to add this for each new fuzzing target.

diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
index 90e75ffaea..8449f81687 100644
--- a/tests/qtest/fuzz/i440fx_fuzz.c
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -138,6 +138,7 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
         i440fx_fuzz_qos(s, Data, Size);
         _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 d08a47e278..a33bd73067 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -122,6 +122,7 @@ static void virtio_net_fork_fuzz(QTestState *s,
         flush_events(s);
         _Exit(0);
     } else {
+        flush_events(s);
         wait(NULL);
     }
 }
@@ -134,6 +135,7 @@ static void virtio_net_fork_fuzz_check_used(QTestState *s,
         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 3b95247f12..51dce491ab 100644
--- a/tests/qtest/fuzz/virtio_scsi_fuzz.c
+++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
@@ -145,6 +145,7 @@ static void virtio_scsi_fork_fuzz(QTestState *s,
         flush_events(s);
         _Exit(0);
     } else {
+        flush_events(s);
         wait(NULL);
     }
 }
@@ -164,6 +165,7 @@ static void virtio_scsi_with_flag_fuzz(QTestState *s,
         }
         _Exit(0);
     } else {
+        flush_events(s);
         wait(NULL);
     }
 }
-- 
2.26.2

Re: [PATCH 1/4] fuzz: add datadir for oss-fuzz compatability

[Darren Kenny]

On Monday, 2020-05-11 at 23:01:30 -04, Alexander Bulekov wrote:
> This allows us to keep pc-bios in executable_dir/pc-bios, rather than
> executable_dir/../pc-bios, which is incompatible with oss-fuzz' file
> structure.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

> ---
>  include/sysemu/sysemu.h |  2 ++
>  softmmu/vl.c            |  2 +-
>  tests/qtest/fuzz/fuzz.c | 15 +++++++++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ef81302e1a..cc96b66fc9 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -15,6 +15,8 @@ extern const char *qemu_name;
>  extern QemuUUID qemu_uuid;
>  extern bool qemu_uuid_set;
>  
> +void qemu_add_data_dir(const char *path);
> +
>  void qemu_add_exit_notifier(Notifier *notify);
>  void qemu_remove_exit_notifier(Notifier *notify);
>  
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index afd2615fb3..c71485a965 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1993,7 +1993,7 @@ char *qemu_find_file(int type, const char *name)
>      return NULL;
>  }
>  
> -static void qemu_add_data_dir(const char *path)
> +void qemu_add_data_dir(const char *path)
>  {
>      int i;
>  
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index f5c923852e..33365c3782 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -137,6 +137,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
>  {
>  
>      char *target_name;
> +    char *dir;
>  
>      /* Initialize qgraph and modules */
>      qos_graph_init();
> @@ -147,6 +148,20 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
>      target_name = strstr(**argv, "-target-");
>      if (target_name) {        /* The binary name specifies the target */
>          target_name += strlen("-target-");
> +        /*
> +         * With oss-fuzz, the executable is kept in the root of a directory (we
> +         * cannot assume the path). All data (including bios binaries) must be
> +         * in the same dir, or a subdir. Thus, we cannot place the pc-bios so
> +         * that it would be in exec_dir/../pc-bios.
> +         * As a workaround, oss-fuzz allows us to use argv[0] to get the
> +         * location of the executable. Using this we add exec_dir/pc-bios to
> +         * the datadirs.
> +         */
> +        dir = g_build_filename(g_path_get_dirname(**argv), "pc-bios", NULL);
> +        if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> +            qemu_add_data_dir(dir);
> +        }
> +        g_free(dir);
>      } else if (*argc > 1) {  /* The target is specified as an argument */
>          target_name = (*argv)[1];
>          if (!strstr(target_name, "--fuzz-target=")) {
> -- 
> 2.26.2

Re: [PATCH 2/4] fuzz: fix typo in i440fx-qtest-reboot arguments

[Darren Kenny]

On Monday, 2020-05-11 at 23:01:31 -04, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

> ---
>  tests/qtest/fuzz/i440fx_fuzz.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
> index ab5f112584..90e75ffaea 100644
> --- a/tests/qtest/fuzz/i440fx_fuzz.c
> +++ b/tests/qtest/fuzz/i440fx_fuzz.c
> @@ -143,7 +143,7 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
>  }
>  
>  static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
> -                                       "-m 0 -display none";
> +                                       " -m 0 -display none";
>  static const char *i440fx_argv(FuzzTarget *t)
>  {
>      return i440fx_qtest_argv;
> -- 
> 2.26.2

Re: [PATCH 4/4] fuzz: run the main-loop in fork-server process

[Darren Kenny]

On Monday, 2020-05-11 at 23:01:33 -04, Alexander Bulekov wrote:
> Without this, the time since the last main-loop keeps increasing, as the
> fuzzer runs. The forked children need to handle all the "past-due"
> timers, slowing them down, over time. With this change, the
> parent/fork-server process runs the main-loop, while waiting on the
> child, ensuring that the timer events do not pile up, over time.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

> ---
>  tests/qtest/fuzz/i440fx_fuzz.c      | 1 +
>  tests/qtest/fuzz/virtio_net_fuzz.c  | 2 ++
>  tests/qtest/fuzz/virtio_scsi_fuzz.c | 2 ++
>  3 files changed, 5 insertions(+)
>
> I'm working on another series to abstract away the details of resetting
> qemu state between runs from the individual targets. That should relieve
> us from needing to add this for each new fuzzing target.
>
> diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
> index 90e75ffaea..8449f81687 100644
> --- a/tests/qtest/fuzz/i440fx_fuzz.c
> +++ b/tests/qtest/fuzz/i440fx_fuzz.c
> @@ -138,6 +138,7 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
>          i440fx_fuzz_qos(s, Data, Size);
>          _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 d08a47e278..a33bd73067 100644
> --- a/tests/qtest/fuzz/virtio_net_fuzz.c
> +++ b/tests/qtest/fuzz/virtio_net_fuzz.c
> @@ -122,6 +122,7 @@ static void virtio_net_fork_fuzz(QTestState *s,
>          flush_events(s);
>          _Exit(0);
>      } else {
> +        flush_events(s);
>          wait(NULL);
>      }
>  }
> @@ -134,6 +135,7 @@ static void virtio_net_fork_fuzz_check_used(QTestState *s,
>          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 3b95247f12..51dce491ab 100644
> --- a/tests/qtest/fuzz/virtio_scsi_fuzz.c
> +++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
> @@ -145,6 +145,7 @@ static void virtio_scsi_fork_fuzz(QTestState *s,
>          flush_events(s);
>          _Exit(0);
>      } else {
> +        flush_events(s);
>          wait(NULL);
>      }
>  }
> @@ -164,6 +165,7 @@ static void virtio_scsi_with_flag_fuzz(QTestState *s,
>          }
>          _Exit(0);
>      } else {
> +        flush_events(s);
>          wait(NULL);
>      }
>  }
> -- 
> 2.26.2

Re: [PATCH 3/4] fuzz: add mangled object name to linker script

[Darren Kenny]

On Monday, 2020-05-11 at 23:01:32 -04, Alexander Bulekov wrote:
> Previously, we relied on "FuzzerTracePC*(.bss*)" to place libfuzzer's
> fuzzer::TPC object into our contiguous shared-memory region. This does
> not work for some libfuzzer builds, so this addition identifies the
> region by its mangled name: *(.bss._ZN6fuzzer3TPCE);
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

FWIW, since I'm not really familiar with the syntax, but I understand
what the intent is:

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>


> ---
>  tests/qtest/fuzz/fork_fuzz.ld | 5 +++++
>  1 file changed, 5 insertions(+)
>
> This isn't ideal, but I looked at the libfuzzer builds packaged for
> debian, for versions 6, 7, 8, 9, 10 and 11 and this (mangled) object
> name appears consistently in the symbol tables.
>
> diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld
> index e086bba873..bfb667ed06 100644
> --- a/tests/qtest/fuzz/fork_fuzz.ld
> +++ b/tests/qtest/fuzz/fork_fuzz.ld
> @@ -28,6 +28,11 @@ SECTIONS
>  
>        /* Internal Libfuzzer TracePC object which contains the ValueProfileMap */
>        FuzzerTracePC*(.bss*);
> +      /*
> +       * In case the above line fails, explicitly specify the (mangled) name of
> +       * the object we care about
> +       */
> +       *(.bss._ZN6fuzzer3TPCE);
>    }
>    .data.fuzz_end : ALIGN(4K)
>    {
> -- 
> 2.26.2

Re: [PATCH 2/4] fuzz: fix typo in i440fx-qtest-reboot arguments

[Philippe Mathieu-Daudé]

On 5/12/20 5:01 AM, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   tests/qtest/fuzz/i440fx_fuzz.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
> index ab5f112584..90e75ffaea 100644
> --- a/tests/qtest/fuzz/i440fx_fuzz.c
> +++ b/tests/qtest/fuzz/i440fx_fuzz.c
> @@ -143,7 +143,7 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
>   }
>   
>   static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
> -                                       "-m 0 -display none";
> +                                       " -m 0 -display none";
>   static const char *i440fx_argv(FuzzTarget *t)
>   {
>       return i440fx_qtest_argv;
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 0/4] fuzz: misc changes for oss-fuzz compatability

[Alexander Bulekov]

ping?

On 200511 2301, Alexander Bulekov wrote:
> Hello,
> With these patches, the fuzzer passes the oss-fuzz build checks.
> There are also some miscelanous improvement to the fuzzer, in general:
>  * If building for oss-fuzz, check executable_dir/pc-bios for
>    the bios images
>  * Fix a typo in the i440fx-qtest-reboot argument which resulted in an
>    invalid argument to qemu_main
>  * Add an alternate name to resolve libfuzzer's internal fuzzer::TPC
>    object at link-time
>  * For all fork-based fuzzers, run the main-loop in the parent, to
>    prevent the clock from running far-ahead of the previous main-loop.
> -Alex
> 
> Alexander Bulekov (4):
>   fuzz: add datadir for oss-fuzz compatability
>   fuzz: fix typo in i440fx-qtest-reboot arguments
>   fuzz: add mangled object name to linker script
>   fuzz: run the main-loop in fork-server process
> 
>  include/sysemu/sysemu.h             |  2 ++
>  softmmu/vl.c                        |  2 +-
>  tests/qtest/fuzz/fork_fuzz.ld       |  5 +++++
>  tests/qtest/fuzz/fuzz.c             | 15 +++++++++++++++
>  tests/qtest/fuzz/i440fx_fuzz.c      |  3 ++-
>  tests/qtest/fuzz/virtio_net_fuzz.c  |  2 ++
>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  2 ++
>  7 files changed, 29 insertions(+), 2 deletions(-)
> 
> -- 
> 2.26.2
> 

Re: [PATCH 1/4] fuzz: add datadir for oss-fuzz compatability

[Philippe Mathieu-Daudé]

On 5/12/20 5:01 AM, Alexander Bulekov wrote:
> This allows us to keep pc-bios in executable_dir/pc-bios, rather than
> executable_dir/../pc-bios, which is incompatible with oss-fuzz' file
> structure.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   include/sysemu/sysemu.h |  2 ++
>   softmmu/vl.c            |  2 +-
>   tests/qtest/fuzz/fuzz.c | 15 +++++++++++++++
>   3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ef81302e1a..cc96b66fc9 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -15,6 +15,8 @@ extern const char *qemu_name;
>   extern QemuUUID qemu_uuid;
>   extern bool qemu_uuid_set;
>   
> +void qemu_add_data_dir(const char *path);
> +
>   void qemu_add_exit_notifier(Notifier *notify);
>   void qemu_remove_exit_notifier(Notifier *notify);
>   
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index afd2615fb3..c71485a965 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1993,7 +1993,7 @@ char *qemu_find_file(int type, const char *name)
>       return NULL;
>   }
>   
> -static void qemu_add_data_dir(const char *path)
> +void qemu_add_data_dir(const char *path)
>   {
>       int i;
>   
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index f5c923852e..33365c3782 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -137,6 +137,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
>   {
>   
>       char *target_name;
> +    char *dir;
>   
>       /* Initialize qgraph and modules */
>       qos_graph_init();
> @@ -147,6 +148,20 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
>       target_name = strstr(**argv, "-target-");
>       if (target_name) {        /* The binary name specifies the target */
>           target_name += strlen("-target-");
> +        /*
> +         * With oss-fuzz, the executable is kept in the root of a directory (we
> +         * cannot assume the path). All data (including bios binaries) must be
> +         * in the same dir, or a subdir. Thus, we cannot place the pc-bios so
> +         * that it would be in exec_dir/../pc-bios.
> +         * As a workaround, oss-fuzz allows us to use argv[0] to get the
> +         * location of the executable. Using this we add exec_dir/pc-bios to
> +         * the datadirs.

I don't understand well the problem. How do you run it?

> +         */
> +        dir = g_build_filename(g_path_get_dirname(**argv), "pc-bios", NULL);
> +        if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> +            qemu_add_data_dir(dir);
> +        }
> +        g_free(dir);
>       } else if (*argc > 1) {  /* The target is specified as an argument */
>           target_name = (*argv)[1];
>           if (!strstr(target_name, "--fuzz-target=")) {
> 

Re: [PATCH 1/4] fuzz: add datadir for oss-fuzz compatability

[Alexander Bulekov]

On 200520 1851, Philippe Mathieu-Daudé wrote:
> On 5/12/20 5:01 AM, Alexander Bulekov wrote:

-snip-
> > +        /*
> > +         * With oss-fuzz, the executable is kept in the root of a directory (we
> > +         * cannot assume the path). All data (including bios binaries) must be
> > +         * in the same dir, or a subdir. Thus, we cannot place the pc-bios so
> > +         * that it would be in exec_dir/../pc-bios.
> > +         * As a workaround, oss-fuzz allows us to use argv[0] to get the
> > +         * location of the executable. Using this we add exec_dir/pc-bios to
> > +         * the datadirs.
> 
> I don't understand well the problem. How do you run it?

On oss-fuzz, the build and execution happens in two separate containers.

1.) In the build container, we can do whatever we want, but we must
place the executable(s) we produce at the root of a directory /out/.
e.g. one output executable is /out/qemu-system-target-i440fx-fuzz

2.) In the runner, this "build artifact" directory is mounted at
some location(we can't assume the location). This runner container
automatically identifies the executable within the root of the  "build
artifact" dir and runs it. The path to the executable could now be 
/somedir/qemu-system-target-i440fx-fuzz

In the runner container we only have control over the files in /somedir/
(which was /out/ in the builder). Thus, in addition to copying over
shared-libs to /out/ we need to copy any data (pc-bios) that the binary
relies on. The problem is that we have to point qemu towards the
location of the bios. Normally qemu checks the /usr/share/... dir. For
local builds, qemu also examines the executable path and looks in
$executable_path/../pc-bios/. On the oss-fuzz runner we dont control
/somedir/../pc-bios, so we can't rely on this. This patch allows us to
specify /somedir/pc-bios as the datadir.

Hopefully that is not too confusing.

For reference, here is a draft of the build-script that I expect to
deploy to oss-fuzz:

# build project
# e.g.
# ./autogen.sh
# ./configure
# make -j$(nproc) all

# build fuzzers
# e.g.
# $CXX $CXXFLAGS -std=c++11 -Iinclude \
#     /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
#     $LIB_FUZZING_ENGINE /path/to/library.a

# make a dir for the shared libs
mkdir -p $OUT/lib/

#Build once to copy over the list of dynamic libs
./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" --extra-cflags="$CFLAGS -U __OPTIMIZE__ "
make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz

# copy over the shared libs
for i in $(ldd ./i386-softmmu/qemu-fuzz-i386  | cut -f3 -d' '); do
    cp $i $OUT/lib/
done
rm ./i386-softmmu/qemu-fuzz-i386

# Build second time to build the final binary with correct rpath
./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" --extra-cflags="$CFLAGS -U __OPTIMIZE__" --extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz

# copy over bios data
cp  -r ./pc-bios/ $OUT/pc-bios
for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/  {print $2}');
do
    cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
done

-Alex
> 
> > +         */
> > +        dir = g_build_filename(g_path_get_dirname(**argv), "pc-bios", NULL);
> > +        if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> > +            qemu_add_data_dir(dir);
> > +        }
> > +        g_free(dir);
> >       } else if (*argc > 1) {  /* The target is specified as an argument */
> >           target_name = (*argv)[1];
> >           if (!strstr(target_name, "--fuzz-target=")) {
> > 
> 

Re: [PATCH 0/4] fuzz: misc changes for oss-fuzz compatability

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

On Mon, May 11, 2020 at 11:01:29PM -0400, Alexander Bulekov wrote:
> Hello,
> With these patches, the fuzzer passes the oss-fuzz build checks.
> There are also some miscelanous improvement to the fuzzer, in general:
>  * If building for oss-fuzz, check executable_dir/pc-bios for
>    the bios images
>  * Fix a typo in the i440fx-qtest-reboot argument which resulted in an
>    invalid argument to qemu_main
>  * Add an alternate name to resolve libfuzzer's internal fuzzer::TPC
>    object at link-time
>  * For all fork-based fuzzers, run the main-loop in the parent, to
>    prevent the clock from running far-ahead of the previous main-loop.
> -Alex
> 
> Alexander Bulekov (4):
>   fuzz: add datadir for oss-fuzz compatability
>   fuzz: fix typo in i440fx-qtest-reboot arguments
>   fuzz: add mangled object name to linker script
>   fuzz: run the main-loop in fork-server process
> 
>  include/sysemu/sysemu.h             |  2 ++
>  softmmu/vl.c                        |  2 +-
>  tests/qtest/fuzz/fork_fuzz.ld       |  5 +++++
>  tests/qtest/fuzz/fuzz.c             | 15 +++++++++++++++
>  tests/qtest/fuzz/i440fx_fuzz.c      |  3 ++-
>  tests/qtest/fuzz/virtio_net_fuzz.c  |  2 ++
>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  2 ++
>  7 files changed, 29 insertions(+), 2 deletions(-)
> 
> -- 
> 2.26.2
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]