[PATCH 0/6] tests/fuzz: Review notes

Philippe Mathieu-Daudé posted 6 patches 5 years, 6 months ago
Test asan passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test FreeBSD passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200514143433.18569-1-philmd@redhat.com
Makefile                          |  6 +++-
tests/qtest/fuzz/i440fx_fuzz.c    | 47 ++++++++++++++++++++-----------
tests/qtest/fuzz/Makefile.include |  6 ++--
3 files changed, 38 insertions(+), 21 deletions(-)
[PATCH 0/6] tests/fuzz: Review notes
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago
I took some code notes while testing Alex's toy.
Sending them as patches.

Alex, I had theses issues while building following
docs/devel/fuzzing.txt:

    CC      nbd/server.o
  nbd/server.c:1937:1: error: unused function 'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
  G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
  ^
  /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
    static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \
                       ^
  /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME'
  #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName
                                                 ^
Solved by using './configure ... --extra-cflags=-Wno-unused-function'

    LINK    i386-softmmu/qemu-fuzz-i386
  /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition
  /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors.cc.o): warning: defined here

I simply ignore this warning.

Alex, could you provide few more examples (in particular about
instantiating devices, or useful libfuzzer arguments)?

Also, I suppose you are using a script to generate the bug reports:

  I can reproduce it in qemu 5.0 using:

  cat << EOF | qemu-system ...
  outl 0xcf8 ...
  outl 0xcfc ...
  ...
  EOF

Is this script available in the QEMU repository? I couldn't find it.

Regards,

Phil.

Philippe Mathieu-Daudé (6):
  tests/fuzz/Makefile: Do not link code using unavailable devices
  Makefile: List fuzz targets in 'make help'
  tests/fuzz: Add missing space in test description
  tests/fuzz: Remove unuseful/unused typedefs
  tests/fuzz: Extract pciconfig_fuzz_qos() method
  tests/fuzz: Extract ioport_fuzz_qtest() method

 Makefile                          |  6 +++-
 tests/qtest/fuzz/i440fx_fuzz.c    | 47 ++++++++++++++++++++-----------
 tests/qtest/fuzz/Makefile.include |  6 ++--
 3 files changed, 38 insertions(+), 21 deletions(-)

-- 
2.21.3


Re: [PATCH 0/6] tests/fuzz: Review notes
Posted by Stefan Hajnoczi 5 years, 6 months ago
On Thu, May 14, 2020 at 04:34:27PM +0200, Philippe Mathieu-Daudé wrote:
> I took some code notes while testing Alex's toy.
> Sending them as patches.
> 
> Alex, I had theses issues while building following
> docs/devel/fuzzing.txt:
> 
>     CC      nbd/server.o
>   nbd/server.c:1937:1: error: unused function 'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
>   G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
>   ^
>   /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
>     static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \
>                        ^
>   /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME'
>   #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName
>                                                  ^
> Solved by using './configure ... --extra-cflags=-Wno-unused-function'
> 
>     LINK    i386-softmmu/qemu-fuzz-i386
>   /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition
>   /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors.cc.o): warning: defined here
> 
> I simply ignore this warning.
> 
> Alex, could you provide few more examples (in particular about
> instantiating devices, or useful libfuzzer arguments)?
> 
> Also, I suppose you are using a script to generate the bug reports:
> 
>   I can reproduce it in qemu 5.0 using:
> 
>   cat << EOF | qemu-system ...
>   outl 0xcf8 ...
>   outl 0xcfc ...
>   ...
>   EOF
> 
> Is this script available in the QEMU repository? I couldn't find it.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (6):
>   tests/fuzz/Makefile: Do not link code using unavailable devices
>   Makefile: List fuzz targets in 'make help'
>   tests/fuzz: Add missing space in test description
>   tests/fuzz: Remove unuseful/unused typedefs
>   tests/fuzz: Extract pciconfig_fuzz_qos() method
>   tests/fuzz: Extract ioport_fuzz_qtest() method
> 
>  Makefile                          |  6 +++-
>  tests/qtest/fuzz/i440fx_fuzz.c    | 47 ++++++++++++++++++++-----------
>  tests/qtest/fuzz/Makefile.include |  6 ++--
>  3 files changed, 38 insertions(+), 21 deletions(-)
> 
> -- 
> 2.21.3
> 

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

Stefan
Re: [PATCH 0/6] tests/fuzz: Review notes
Posted by Alexander Bulekov 5 years, 6 months ago
On 200514 1634, Philippe Mathieu-Daudé wrote:
> I took some code notes while testing Alex's toy.
> Sending them as patches.

Thank you for these! I'd send my review tags, if they weren't already
pulled.

> Alex, I had theses issues while building following
> docs/devel/fuzzing.txt:
> 
>     CC      nbd/server.o
>   nbd/server.c:1937:1: error: unused function 'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
>   G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
>   ^
>   /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
>     static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \
>                        ^
>   /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME'
>   #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName
>                                                  ^
> Solved by using './configure ... --extra-cflags=-Wno-unused-function'

Interesting that this doesn't show up for standard builds. I'll have to
look into this more.

>     LINK    i386-softmmu/qemu-fuzz-i386
>   /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition
>   /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors.cc.o): warning: defined here
> 
> I simply ignore this warning.

Yes, I get the same warning with this:

cat << EOF | clang-9 -Wl,--warn-common  -fsanitize=fuzzer,address -x c -
#include <sys/types.h>
#include <stdint.h>
int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { return 0; }
EOF

Might report this to the LLVM/compiler-rt folks.

> Alex, could you provide few more examples (in particular about
> instantiating devices, or useful libfuzzer arguments)?

Yes - I'll send this out with my next fuzzing patches.

> Also, I suppose you are using a script to generate the bug reports:
> 
>   I can reproduce it in qemu 5.0 using:
> 
>   cat << EOF | qemu-system ...
>   outl 0xcf8 ...
>   outl 0xcfc ...
>   ...
>   EOF
> 
> Is this script available in the QEMU repository? I couldn't find it.

Right now I just added some unbuffered writes to qtest.c. I'll try to
find a clean way of doing this and I'll add a way to automatically
convert a crashing input to qemu arguments + qtest command sequence.

Thanks again!
-Alex

> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (6):
>   tests/fuzz/Makefile: Do not link code using unavailable devices
>   Makefile: List fuzz targets in 'make help'
>   tests/fuzz: Add missing space in test description
>   tests/fuzz: Remove unuseful/unused typedefs
>   tests/fuzz: Extract pciconfig_fuzz_qos() method
>   tests/fuzz: Extract ioport_fuzz_qtest() method
> 
>  Makefile                          |  6 +++-
>  tests/qtest/fuzz/i440fx_fuzz.c    | 47 ++++++++++++++++++++-----------
>  tests/qtest/fuzz/Makefile.include |  6 ++--
>  3 files changed, 38 insertions(+), 21 deletions(-)
> 
> -- 
> 2.21.3
> 

Re: [PATCH 0/6] tests/fuzz: Review notes
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago
On 5/15/20 5:10 PM, Alexander Bulekov wrote:
> On 200514 1634, Philippe Mathieu-Daudé wrote:
>> I took some code notes while testing Alex's toy.
>> Sending them as patches.
> 
> Thank you for these! I'd send my review tags, if they weren't already
> pulled.

The series is queued but not pulled. Review tags are always welcome, and 
maintainers might add them after-queue before-pull when they have time.

> 
>> Alex, I had theses issues while building following
>> docs/devel/fuzzing.txt:
>>
>>      CC      nbd/server.o
>>    nbd/server.c:1937:1: error: unused function 'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
>>    G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
>>    ^
>>    /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
>>      static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \
>>                         ^
>>    /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME'
>>    #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName
>>                                                   ^
>> Solved by using './configure ... --extra-cflags=-Wno-unused-function'
> 
> Interesting that this doesn't show up for standard builds. I'll have to
> look into this more.

I don't think it is related to your work.

> 
>>      LINK    i386-softmmu/qemu-fuzz-i386
>>    /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition
>>    /usr/lib64/clang/9.0.1/lib/linux/libclang_rt.asan-aarch64.a(asan_interceptors.cc.o): warning: defined here
>>
>> I simply ignore this warning.
> 
> Yes, I get the same warning with this:
> 
> cat << EOF | clang-9 -Wl,--warn-common  -fsanitize=fuzzer,address -x c -
> #include <sys/types.h>
> #include <stdint.h>
> int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { return 0; }
> EOF
> 
> Might report this to the LLVM/compiler-rt folks.
> 
>> Alex, could you provide few more examples (in particular about
>> instantiating devices, or useful libfuzzer arguments)?
> 
> Yes - I'll send this out with my next fuzzing patches.
> 
>> Also, I suppose you are using a script to generate the bug reports:
>>
>>    I can reproduce it in qemu 5.0 using:
>>
>>    cat << EOF | qemu-system ...
>>    outl 0xcf8 ...
>>    outl 0xcfc ...
>>    ...
>>    EOF
>>
>> Is this script available in the QEMU repository? I couldn't find it.
> 
> Right now I just added some unbuffered writes to qtest.c. I'll try to
> find a clean way of doing this and I'll add a way to automatically
> convert a crashing input to qemu arguments + qtest command sequence.
> 
> Thanks again!

You are welcomed, waiting for your following series :)

> -Alex
> 
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (6):
>>    tests/fuzz/Makefile: Do not link code using unavailable devices
>>    Makefile: List fuzz targets in 'make help'
>>    tests/fuzz: Add missing space in test description
>>    tests/fuzz: Remove unuseful/unused typedefs
>>    tests/fuzz: Extract pciconfig_fuzz_qos() method
>>    tests/fuzz: Extract ioport_fuzz_qtest() method
>>
>>   Makefile                          |  6 +++-
>>   tests/qtest/fuzz/i440fx_fuzz.c    | 47 ++++++++++++++++++++-----------
>>   tests/qtest/fuzz/Makefile.include |  6 ++--
>>   3 files changed, 38 insertions(+), 21 deletions(-)
>>
>> -- 
>> 2.21.3
>>
>