|
[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 |