[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target
On Mon, Dec 12, 2016 at 02:58:39AM -0700, Jan Beulich wrote: > >>> On 12.12.16 at 10:28, <wei.liu2@xxxxxxxxxx> wrote: > > Instruction emulator fuzzing code is from code previous written by > > Andrew and George. Adapted to llvm fuzzer and hook up the build system. > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > > Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx> > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx> > > --- > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx> > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> > > Cc: Jan Beulich <jbeulich@xxxxxxxx> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx> > > Cc: Tim Deegan <tim@xxxxxxx> > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx> > > > > v3: > > 1. coding style fix > > 2. share more code > > 3. exit when stack can't be made executable > > --- > > .gitignore | 1 + > > tools/fuzz/x86_instruction_emulator/Makefile | 31 ++++ > > .../x86-insn-emulator-fuzzer.c | 195 > > +++++++++++++++++++++ > > 3 files changed, 227 insertions(+) > > create mode 100644 tools/fuzz/x86_instruction_emulator/Makefile > > create mode 100644 > > tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c > > > > diff --git a/.gitignore b/.gitignore > > index a2f34a1..d507243 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -145,6 +145,7 @@ tools/flask/utils/flask-loadpolicy > > tools/flask/utils/flask-setenforce > > tools/flask/utils/flask-set-bool > > tools/flask/utils/flask-label-pci > > +tools/fuzz/x86_instruction_emulator/x86_emulate* > > tools/helpers/_paths.h > > tools/helpers/init-xenstore-domain > > tools/helpers/xen-init-dom0 > > diff --git a/tools/fuzz/x86_instruction_emulator/Makefile > > b/tools/fuzz/x86_instruction_emulator/Makefile > > new file mode 100644 > > index 0000000..2b147ac > > --- /dev/null > > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > > @@ -0,0 +1,31 @@ > > +XEN_ROOT=$(CURDIR)/../../.. > > +include $(XEN_ROOT)/tools/Rules.mk > > + > > +x86-instruction-emulator-fuzzer-all: x86-insn-emulator.a > > x86-insn-emulator-fuzzer.o > > + > > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > > + [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > > + > > +x86_emulate.c x86_emulate.h: %: > > + [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* > > + > > +CFLAGS += $(CFLAGS_xeninclude) > > + > > +x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c > > x86_emulate/x86_emulate.h > > Perhaps worthwhile shortening this to > > x86_emulate.o: x86_emulate.[ch] x86_emulate/x86_emulate.[ch] > > ? Done. > > > --- /dev/null > > +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c > > @@ -0,0 +1,195 @@ > > +#include <assert.h> > > +#include <errno.h> > > +#include <fcntl.h> > > +#include <inttypes.h> > > +#include <limits.h> > > +#include <stdbool.h> > > +#include <stdint.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <sys/mman.h> > > +#include <unistd.h> > > +#include <xen/xen.h> > > + > > +#include "x86_emulate.h" > > + > > +static unsigned char data[4096]; > > +static unsigned int data_index = 0; > > Pointless initializer. > Done. > > +static unsigned int data_max; > > + > > +static int data_read(const char *why, void *dst, unsigned int bytes) > > +{ > > + unsigned i; > > Please don't omit the "int" here (and in a few more places below) > when basically everywhere else it is present. > Done. > > + if ( data_index + bytes > data_max ) > > + return X86EMUL_EXCEPTION; > > + > > + memcpy(dst, data+data_index, bytes); > > Blanks around binary operators please (more further down). > Done. > > + data_index += bytes; > > + > > + printf("%s: ", why); > > + for ( i = 0; i < bytes; i++ ) > > + printf(" %02x", (unsigned int)*(unsigned char *)(dst+i)); > > Is the left most cast really needed here? > No. I've deleted that. > > +int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size) > > +{ > > + bool stack_exec; > > + struct cpu_user_regs regs = {}; > > + struct x86_emulate_ctxt ctxt = > > + { > > + .regs = ®s, > > + .addr_size = 8 * sizeof(void *), > > + .sp_size = 8 * sizeof(void *), > > + }; > > + > > Stray blank line. The indentation of the initializer above also looks > a little unusual. > Fixed. > > + unsigned nr = 0; > > + int rc; > > + unsigned x; > > + const uint8_t *p = data_p; > > + > > + stack_exec = emul_test_make_stack_executable(); > > + if ( !stack_exec ) > > + { > > + printf("Warning: Stack could not be made executable (%d).\n", > > errno); > > + exit(1); > > + } > > + > > + /* Reset all global states */ > > DYM "state"? > I mean "states". There are three states we need to reset. > > + memset(data, 0, sizeof(data)); > > + data_index = 0; > > + data_max = 0; > > + > > + nr = size < sizeof(regs) ? size : sizeof(regs); > > + > > + memcpy(®s, p, nr); > > + p += sizeof(regs); > > + nr += sizeof(regs); > > I think this second += wants to be dropped, considering how nr > gets set above and used below. > > > + if ( nr <= size ) > > < would seem more natural here. Yes, you're right in both places. > > > + { > > + memcpy(data, p, size - nr); > > + data_max = size - nr; > > + } > > + > > + ctxt.force_writeback = 0; > > false Done. > > > + /* Zero 'private' entries */ > > s/entries/fields/ ? > Done. > > + regs.error_code = 0; > > + regs.entry_vector = 0; > > + > > + /* Use the upper bits of regs.eip to determine addr_size */ > > + x = (regs.rip >> ADDR_SIZE_SHIFT) & 0x3; > > This won't build as 32-bit code. If that's intentional, then I think > this would better be enforced in the Makefile (rather than > surfacing a compile error here). > Good catch. I think this test case is still preliminary. TBH I haven't paid much attention to the working of this test target, other than pulling everything together to work. I think long term we do need to determine what to do with 32 bit build, but I would wait until George to come back because he wrote this snippet. For now I will disable 32bit build in Makefile. > > + if (x == 3) I also fix this instance to add spaces in (). ---8<--- From 83b7381080aafc3f2fb35ba589715694f847f73a Mon Sep 17 00:00:00 2001 From: Wei Liu <wei.liu2@xxxxxxxxxx> Date: Thu, 8 Dec 2016 12:09:54 +0000 Subject: [PATCH] tools/fuzz: introduce x86 instruction emulator target Instruction emulator fuzzing code is from code previous written by Andrew and George. Adapt it to llvm fuzzer and hook up the build system. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx> --- Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Jan Beulich <jbeulich@xxxxxxxx> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx> Cc: Tim Deegan <tim@xxxxxxx> Cc: Wei Liu <wei.liu2@xxxxxxxxxx> v4: 1. more coding style fixes and bug fixes 2. only do 64 bit build v3: 1. coding style fix 2. share more code 3. exit when stack can't be made executable --- .gitignore | 1 + tools/fuzz/x86_instruction_emulator/Makefile | 36 ++++ .../x86-insn-emulator-fuzzer.c | 190 +++++++++++++++++++++ 3 files changed, 227 insertions(+) create mode 100644 tools/fuzz/x86_instruction_emulator/Makefile create mode 100644 tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c diff --git a/.gitignore b/.gitignore index a2f34a1..d507243 100644 --- a/.gitignore +++ b/.gitignore @@ -145,6 +145,7 @@ tools/flask/utils/flask-loadpolicy tools/flask/utils/flask-setenforce tools/flask/utils/flask-set-bool tools/flask/utils/flask-label-pci +tools/fuzz/x86_instruction_emulator/x86_emulate* tools/helpers/_paths.h tools/helpers/init-xenstore-domain tools/helpers/xen-init-dom0 diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile new file mode 100644 index 0000000..6e68df7 --- /dev/null +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -0,0 +1,36 @@ +XEN_ROOT=$(CURDIR)/../../.. +include $(XEN_ROOT)/tools/Rules.mk + +.PHONY: x86-instruction-emulator-fuzzer-all +ifeq ($(CONFIG_X86_64),y) +x86-instruction-emulator-fuzzer-all: x86-insn-emulator.a x86-insn-emulator-fuzzer.o +else +x86-instruction-emulator-fuzzer-all: +endif + +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: + [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . + +x86_emulate.c x86_emulate.h: %: + [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* + +CFLAGS += $(CFLAGS_xeninclude) + +x86_emulate.o: x86_emulate.[ch] x86_emulate/x86_emulate.[ch] + +x86-insn-emulator.a: x86_emulate.o + $(AR) rc $@ $^ + +x86-insn-emulator-fuzzer.o: x86-insn-emulator-fuzzer.c + +# Common targets +.PHONY: all +all: x86-instruction-emulator-fuzzer-all + +.PHONY: distclean +distclean: clean + rm -f x86_emulate x86_emulate.c x86_emulate.h + +.PHONY: clean +clean: + rm -f *.a *.o diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c new file mode 100644 index 0000000..94ec311 --- /dev/null +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c @@ -0,0 +1,190 @@ +#include <assert.h> +#include <errno.h> +#include <fcntl.h> +#include <inttypes.h> +#include <limits.h> +#include <stdbool.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/mman.h> +#include <unistd.h> +#include <xen/xen.h> + +#include "x86_emulate.h" + +static unsigned char data[4096]; +static unsigned int data_index; +static unsigned int data_max; + +static int data_read(const char *why, void *dst, unsigned int bytes) +{ + unsigned int i; + + if ( data_index + bytes > data_max ) + return X86EMUL_EXCEPTION; + + memcpy(dst, data + data_index, bytes); + data_index += bytes; + + printf("%s: ", why); + for ( i = 0; i < bytes; i++ ) + printf(" %02x", *(unsigned char *)(dst + i)); + printf("\n"); + + return X86EMUL_OKAY; +} + +static int fuzz_read( + unsigned int seg, + unsigned long offset, + void *p_data, + unsigned int bytes, + struct x86_emulate_ctxt *ctxt) +{ + return data_read("read", p_data, bytes); +} + +static int fuzz_fetch( + unsigned int seg, + unsigned long offset, + void *p_data, + unsigned int bytes, + struct x86_emulate_ctxt *ctxt) +{ + return data_read("fetch", p_data, bytes); +} + +static int fuzz_write( + unsigned int seg, + unsigned long offset, + void *p_data, + unsigned int bytes, + struct x86_emulate_ctxt *ctxt) +{ + return X86EMUL_OKAY; +} + +static int fuzz_cmpxchg( + unsigned int seg, + unsigned long offset, + void *old, + void *new, + unsigned int bytes, + struct x86_emulate_ctxt *ctxt) +{ + return X86EMUL_OKAY; +} + +static struct x86_emulate_ops fuzz_emulops = { + .read = fuzz_read, + .insn_fetch = fuzz_fetch, + .write = fuzz_write, + .cmpxchg = fuzz_cmpxchg, + .cpuid = emul_test_cpuid, + .read_cr = emul_test_read_cr, + .get_fpu = emul_test_get_fpu, +}; + +#define CANONICALIZE(x) \ + do { \ + uint64_t _y = (x); \ + if ( _y & (1ULL << 47) ) \ + _y |= (~0ULL) << 48; \ + else \ + _y &= (1ULL << 48)-1; \ + printf("Canonicalized %" PRIx64 " to %" PRIx64 "\n", x, _y); \ + (x) = _y; \ + } while( 0 ) + +#define ADDR_SIZE_SHIFT 60 +#define ADDR_SIZE_64 (2ULL << ADDR_SIZE_SHIFT) +#define ADDR_SIZE_32 (1ULL << ADDR_SIZE_SHIFT) +#define ADDR_SIZE_16 (0) + +int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size) +{ + bool stack_exec; + struct cpu_user_regs regs = {}; + struct x86_emulate_ctxt ctxt = { + .regs = ®s, + .addr_size = 8 * sizeof(void *), + .sp_size = 8 * sizeof(void *), + }; + unsigned int nr = 0; + int rc; + unsigned int x; + const uint8_t *p = data_p; + + stack_exec = emul_test_make_stack_executable(); + if ( !stack_exec ) + { + printf("Warning: Stack could not be made executable (%d).\n", errno); + return 1; + } + + /* Reset all global states */ + memset(data, 0, sizeof(data)); + data_index = 0; + data_max = 0; + + nr = size < sizeof(regs) ? size : sizeof(regs); + + memcpy(®s, p, nr); + p += sizeof(regs); + + if ( nr < size ) + { + memcpy(data, p, size - nr); + data_max = size - nr; + } + + ctxt.force_writeback = false; + + /* Zero 'private' fields */ + regs.error_code = 0; + regs.entry_vector = 0; + + /* Use the upper bits of regs.eip to determine addr_size */ + x = (regs.rip >> ADDR_SIZE_SHIFT) & 0x3; + if ( x == 3 ) + x = 2; + ctxt.addr_size = 16 << x; + printf("addr_size: %d\n", ctxt.addr_size); + + /* Use the upper bit of regs.rsp to determine sp_size (if appropriate) */ + if ( ctxt.addr_size == 64 ) + ctxt.sp_size = 64; + else + { + /* If addr_size isn't 64-bits, sp_size can only be 16 or 32 bits */ + x = (regs.rsp >> ADDR_SIZE_SHIFT) & 0x1; + ctxt.sp_size = 16 << x; + } + printf("sp_size: %d\n", ctxt.sp_size); + CANONICALIZE(regs.rip); + CANONICALIZE(regs.rsp); + CANONICALIZE(regs.rbp); + + /* Zero all segments for now */ + regs.cs = regs.ss = regs.es = regs.ds = regs.fs = regs.gs = 0; + + do { + rc = x86_emulate(&ctxt, &fuzz_emulops); + printf("Emulation result: %d\n", rc); + } while ( rc == X86EMUL_OKAY ); + + return 0; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |