Re: [PATCH v4 0/5] x86 instruction emulator fuzzing

[Paolo Bonzini]

On 28/06/19 11:33, Alexander Graf wrote:
> 
> 
> On 28.06.19 11:26, Sam Caccavale wrote:
>> Dear all,
>>
>> This series aims to provide an entrypoint for, and fuzz KVM's x86
>> instruction
>> emulator from userspace. It mirrors Xen's application of the AFL
>> fuzzer to
>> it's instruction emulator in the hopes of discovering vulnerabilities.
>> Since this entrypoint also allows arbitrary execution of the emulators
>> code
>> from userspace, it may also be useful for testing.
>>
>> The current 4 patches build the emulator and 2 harnesses:
>> simple-harness is
>> an example of unit testing; afl-harness is a frontend for the AFL fuzzer.
>> The fifth patch contains useful scripts for development but is not
>> intended
>> for usptream consumption.
>>
>> Patches
>> =======
>>
>> - 01: Builds and links afl-harness with the required kernel objects.
>> - 02: Introduces the minimal set of emulator operations and supporting
>> code
>> to emulate simple instructions.
>> - 03: Demonstrates simple-harness as a unit test.
>> - 04: Adds scripts for install and building.
>> - 05: Useful scripts for development
>>
>>
>> Issues
>> =======
>>
>> Currently, fuzzing results in a large amount of FPU related crashes.Â
>> Xen's
>> fuzzing efforts had this issue too. Their (temporary?) solution was to
>> disable FPU exceptions after every instruction iteration? Some solution
>> is desired for this project.
>>
>>
>> Changelog
>> =======
>>
>> v1 -> v2:
>> Â - Moved -O0 to ifdef DEBUG
>> Â - Building with ASAN by default
>> Â - Removed a number of macros from emulator_ops.c and moved them as
>> ÂÂÂ static inline functions in emulator_ops.h
>> Â - Accidentally changed the example in simple-harness (reverted in v3)
>> Â - Introduced patch 4 for scripts
>>
>> v2 -> v3:
>> Â - Removed a workaround for printf smashing the stack when compiled
>> ÂÂÂ with -mcmodel=kernel, and stopped compiling with -mcmodel=kernel
>> Â - Added a null check for malloc's return value
>> Â - Moved more macros from emulator_ops.c into emulator_ops.h as
>> ÂÂÂ static inline functions
>> Â - Removed commented out code
>> Â - Moved changes to emulator_ops.h into the first patch
>> Â - Moved addition of afl-many script to the script patch
>> Â - Fixed spelling mistakes in documentation
>> Â - Reverted the simple-harness example back to the more useful
>> original one
>> Â - Moved non-essential development scripts from patch 4 to new patch 5
>>
>> v3 -> v4:
>> Â - Stubbed out all unimplemented emulator_ops with a unimplemented_op
>> macro
>> Â - Setting FAIL_ON_UNIMPLEMENTED_OP on compile decides whether
>> calling these
>> ÂÂÂ is treated as a crash or ignored
>> Â - Moved setting up core dumps out of the default build/install path and
>> ÂÂÂ detailed this change in the README
>> Â - Added a .sh extention to afl-many
>> Â - Added an optional timeout to afl-many.sh and made deploy_remote.sh
>> use it
>> Â - Building no longer creates a new .config each time and does not
>> force any
>> ÂÂÂ config options
>> Â - Fixed a path bug in afl-many.sh
>>
>> Any comments/suggestions are greatly appreciated.
>>
>> Best,
>> Sam Caccavale
>>
>> Sam Caccavale (5):
>> ÂÂ Build target for emulate.o as a userspace binary
>> ÂÂ Emulate simple x86 instructions in userspace
>> ÂÂ Demonstrating unit testing via simple-harness
>> ÂÂ Added build and install scripts
>> ÂÂ Development scripts for crash triage and deploy
>>
>> Â tools/MakefileÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 9 +
>> Â tools/fuzz/x86ie/.gitignoreÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 2 +
>> Â tools/fuzz/x86ie/MakefileÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 54 ++
>> Â tools/fuzz/x86ie/README.mdÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 21 +
>> Â tools/fuzz/x86ie/afl-harness.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ | 151 +++++
>> Â tools/fuzz/x86ie/common.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 87 +++
>> Â tools/fuzz/x86ie/emulator_ops.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂ | 590 ++++++++++++++++++
>> Â tools/fuzz/x86ie/emulator_ops.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂ | 134 ++++
>> Â tools/fuzz/x86ie/scripts/afl-many.shÂÂÂÂÂÂÂÂÂ |Â 31 +
>> Â tools/fuzz/x86ie/scripts/bin.shÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 49 ++
>> Â tools/fuzz/x86ie/scripts/build.shÂÂÂÂÂÂÂÂÂÂÂÂ |Â 34 +
>> Â tools/fuzz/x86ie/scripts/coalesce.shÂÂÂÂÂÂÂÂÂ |ÂÂ 5 +
>> Â tools/fuzz/x86ie/scripts/deploy.shÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 9 +
>> Â tools/fuzz/x86ie/scripts/deploy_remote.shÂÂÂÂ |Â 10 +
>> Â tools/fuzz/x86ie/scripts/gen_output.shÂÂÂÂÂÂÂ |Â 11 +
>> Â tools/fuzz/x86ie/scripts/install_afl.shÂÂÂÂÂÂ |Â 15 +
>> Â .../fuzz/x86ie/scripts/install_deps_ubuntu.sh |ÂÂ 5 +
>> Â tools/fuzz/x86ie/scripts/rebuild.shÂÂÂÂÂÂÂÂÂÂ |ÂÂ 6 +
>> Â tools/fuzz/x86ie/scripts/run.shÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 10 +
>> Â tools/fuzz/x86ie/scripts/summarize.shÂÂÂÂÂÂÂÂ |ÂÂ 9 +
>> Â tools/fuzz/x86ie/simple-harness.cÂÂÂÂÂÂÂÂÂÂÂÂ |Â 49 ++
>> Â tools/fuzz/x86ie/stubs.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 59 ++
>> Â tools/fuzz/x86ie/stubs.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 52 ++
> 
> Sorry I didn't realize it before. Isn't that missing a patch to the
> MAINTAINERS file?

Yeah, and the directory should probably be tools/fuzz/kvm_emulate so as
not to puzzle people.  Also:

- let's limit the scripts to the minimum, i.e. only the run script which
should be something like

#!/bin/bash
# SPDX-License-Identifier: GPL-2.0+

FUZZDIR="${FUZZDIR:-$(pwd)/fuzz}"

mkdir -p $FUZZDIR/in
cp tools/fuzz/kvm_emulate/rand_sample.bin $FUZZDIR/in
mkdir -p $FUZZDIR/out

${TIMEOUT:+TIMEOUT=$TIMEOUT} ${AFL_FUZZ-afl-fuzz} "$@" \
  -i $FUZZDIR/in -o $FUZZDIR/out tools/fuzz/kvm_emulate/afl-harness @@

where people can substitute afl-many.sh or add their own options using
the AFL_FUZZ variable or the command line.  Likewise for screen.

- the build should be just "make -C tools/fuzz/kvm_emulate" and it
should just work.  Feel free to steal the Makefile magic from other
tools/ directories.

- finally, rand_sample.bin is missing.

Otherwise, it looks very nice.

Paolo