[PATCH v3 0/7] Fuzzing targets for oss-fuzz

[PATCH v3 0/7] Fuzzing targets for oss-fuzz

[Wei Liu]

Hi all

This series adds two fuzzing targets to run in Google's oss-fuzz
infrastructure.

There will be some other patches on the oss-fuzz side. Their recommendation is
to have all the fuzzing targets committed in our tree so that they can be
kept up to date.

Please see the patch to add README for details on how this works.

Wei.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>


Wei Liu (7):
  tools/fuzz: introduce libelf target
  x86emul/test: factor out emul_test_make_stack_executable
  x86emul/test: factor out emul_test_{read_cr,cpuid}
  x86emul/test: factor out emul_test_get_fpu
  tools/fuzz: introduce x86 instruction emulator target
  tools: hook up fuzz directory
  tools/fuzz: add README

 .gitignore                                         |   1 +
 tools/Makefile                                     |   1 +
 tools/fuzz/Makefile                                |  11 ++
 tools/fuzz/README                                  |  39 +++++
 tools/fuzz/libelf/Makefile                         |  31 ++++
 tools/fuzz/libelf/libelf-fuzzer.c                  |  32 ++++
 tools/fuzz/x86_instruction_emulator/Makefile       |  31 ++++
 .../x86-insn-emulator-fuzzer.c                     | 195 +++++++++++++++++++++
 tools/tests/x86_emulator/test_x86_emulator.c       | 142 +--------------
 tools/tests/x86_emulator/x86_emulate.c             |  84 +++++++++
 tools/tests/x86_emulator/x86_emulate.h             |  81 +++++++++
 xen/common/libelf/libelf-private.h                 |   2 +
 12 files changed, 513 insertions(+), 137 deletions(-)
 create mode 100644 tools/fuzz/Makefile
 create mode 100644 tools/fuzz/README
 create mode 100644 tools/fuzz/libelf/Makefile
 create mode 100644 tools/fuzz/libelf/libelf-fuzzer.c
 create mode 100644 tools/fuzz/x86_instruction_emulator/Makefile
 create mode 100644 tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 1/7] tools/fuzz: introduce libelf target

[Wei Liu]

Source code and Makefile to fuzz libelf in Google's oss-fuzz
infrastructure.

Introduce FUZZ_NO_LIBXC in libelf-private.h. That macro will be set when
compiling libelf fuzzer target because libxc is not required in libelf
fuzzing.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/fuzz/libelf/Makefile         | 31 +++++++++++++++++++++++++++++++
 tools/fuzz/libelf/libelf-fuzzer.c  | 32 ++++++++++++++++++++++++++++++++
 xen/common/libelf/libelf-private.h |  2 ++
 3 files changed, 65 insertions(+)
 create mode 100644 tools/fuzz/libelf/Makefile
 create mode 100644 tools/fuzz/libelf/libelf-fuzzer.c

diff --git a/tools/fuzz/libelf/Makefile b/tools/fuzz/libelf/Makefile
new file mode 100644
index 0000000..0e9d40a
--- /dev/null
+++ b/tools/fuzz/libelf/Makefile
@@ -0,0 +1,31 @@
+XEN_ROOT = $(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+# libelf fuzz target
+vpath %.c ../../../xen/common/libelf
+CFLAGS += -I../../../xen/common/libelf
+ELF_SRCS-y += libelf-tools.c libelf-loader.c libelf-dominfo.c
+ELF_LIB_OBJS := $(patsubst %.c,%.o,$(ELF_SRCS-y))
+
+$(patsubst %.c,%.o,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
+
+$(ELF_LIB_OBJS): CFLAGS += -DFUZZ_NO_LIBXC $(CFLAGS_xeninclude)
+
+libelf-fuzzer.o: CFLAGS += $(CFLAGS_xeninclude)
+
+libelf.a: $(ELF_LIB_OBJS)
+	$(AR) rc $@ $^
+
+.PHONY: libelf-fuzzer-all
+libelf-fuzzer-all: libelf.a libelf-fuzzer.o
+
+# Common targets
+.PHONY: all
+all: libelf-fuzzer-all
+
+.PHONY: distclean
+distclean: clean
+
+.PHONY: clean
+clean:
+	rm -f *.o *.a
diff --git a/tools/fuzz/libelf/libelf-fuzzer.c b/tools/fuzz/libelf/libelf-fuzzer.c
new file mode 100644
index 0000000..71561d3
--- /dev/null
+++ b/tools/fuzz/libelf/libelf-fuzzer.c
@@ -0,0 +1,32 @@
+#include <inttypes.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <xen/libelf/libelf.h>
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+    struct elf_binary elf_buf, *elf;
+    struct elf_dom_parms parms;
+
+    elf = &elf_buf;
+
+    memset(elf, 0, sizeof(*elf));
+    elf_init(elf, (const char *)data, size);
+    elf_parse_binary(elf);
+    elf_xen_parse(elf, &parms);
+
+    return 0;
+}
+
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/libelf/libelf-private.h b/xen/common/libelf/libelf-private.h
index 388c3da..47db679 100644
--- a/xen/common/libelf/libelf-private.h
+++ b/xen/common/libelf/libelf-private.h
@@ -72,8 +72,10 @@
 #include <xen/elfnote.h>
 #include <xen/libelf/libelf.h>
 
+#ifndef FUZZ_NO_LIBXC
 #include "xenctrl.h"
 #include "xc_private.h"
+#endif
 
 #define elf_msg(elf, fmt, args ... )                    \
     elf_call_log_callback(elf, 0, fmt , ## args );
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 2/7] x86emul/test: factor out emul_test_make_stack_executable

[Wei Liu]

It will be used by emulator fuzzing target.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 tools/tests/x86_emulator/test_x86_emulator.c | 12 ++----------
 tools/tests/x86_emulator/x86_emulate.c       | 20 ++++++++++++++++++++
 tools/tests/x86_emulator/x86_emulate.h       |  3 +++
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index eed8a0d..0d80bff 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -23,8 +23,6 @@ static const struct {
 #endif
 };
 
-#define MMAP_SZ 16384
-
 /* EFLAGS bit definitions. */
 #define EFLG_OF (1<<11)
 #define EFLG_DF (1<<10)
@@ -234,7 +232,6 @@ int main(int argc, char **argv)
     struct cpu_user_regs regs;
     char *instr;
     unsigned int *res, i, j;
-    unsigned long sp;
     bool stack_exec;
     int rc;
 #ifndef __x86_64__
@@ -258,13 +255,8 @@ int main(int argc, char **argv)
     }
     instr = (char *)res + 0x100;
 
-#ifdef __x86_64__
-    asm ("movq %%rsp, %0" : "=g" (sp));
-#else
-    asm ("movl %%esp, %0" : "=g" (sp));
-#endif
-    stack_exec = mprotect((void *)(sp & -0x1000L) - (MMAP_SZ - 0x1000),
-                          MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC) == 0;
+    stack_exec = emul_test_make_stack_executable();
+
     if ( !stack_exec )
         printf("Warning: Stack could not be made executable (%d).\n", errno);
 
diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index 66c2464..963dd71 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -1,5 +1,7 @@
 #include "x86_emulate.h"
 
+#include <sys/mman.h>
+
 #define EFER_SCE       (1 << 0)
 #define EFER_LMA       (1 << 10)
 
@@ -18,4 +20,22 @@
 #define get_stub(stb) ((void *)((stb).addr = (uintptr_t)(stb).buf))
 #define put_stub(stb)
 
+bool emul_test_make_stack_executable(void)
+{
+    unsigned long sp;
+
+    /*
+     * Mark the entire stack executable so that the stub executions
+     * don't fault
+     */
+#ifdef __x86_64__
+    asm ("movq %%rsp, %0" : "=g" (sp));
+#else
+    asm ("movl %%esp, %0" : "=g" (sp));
+#endif
+
+    return mprotect((void *)(sp & -0x1000L) - (MMAP_SZ - 0x1000),
+                    MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC) == 0;
+}
+
 #include "x86_emulate/x86_emulate.c"
diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h
index 1981326..a9b874c 100644
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -33,4 +33,7 @@
 
 #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
 
+#define MMAP_SZ 16384
+bool emul_test_make_stack_executable(void);
+
 #include "x86_emulate/x86_emulate.h"
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 3/7] x86emul/test: factor out emul_test_{read_cr, cpuid}

[Wei Liu]

While at it, move xgetbv, all cpu_has_* and cache_line_size macros to
x86_emulate.h.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/tests/x86_emulator/test_x86_emulator.c | 103 +--------------------------
 tools/tests/x86_emulator/x86_emulate.c       |  39 ++++++++++
 tools/tests/x86_emulator/x86_emulate.h       |  72 +++++++++++++++++++
 3 files changed, 113 insertions(+), 101 deletions(-)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index 0d80bff..e40f0ea 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -92,105 +92,6 @@ static int cmpxchg(
     return X86EMUL_OKAY;
 }
 
-static int cpuid(
-    unsigned int *eax,
-    unsigned int *ebx,
-    unsigned int *ecx,
-    unsigned int *edx,
-    struct x86_emulate_ctxt *ctxt)
-{
-    unsigned int leaf = *eax;
-
-    asm ("cpuid" : "+a" (*eax), "+c" (*ecx), "=d" (*edx), "=b" (*ebx));
-
-    /* The emulator doesn't itself use MOVBE, so we can always run the test. */
-    if ( leaf == 1 )
-        *ecx |= 1U << 22;
-
-    return X86EMUL_OKAY;
-}
-
-#define cache_line_size() ({ \
-    unsigned int eax = 1, ebx, ecx = 0, edx; \
-    cpuid(&eax, &ebx, &ecx, &edx, NULL); \
-    edx & (1U << 19) ? (ebx >> 5) & 0x7f8 : 0; \
-})
-
-#define cpu_has_mmx ({ \
-    unsigned int eax = 1, ecx = 0, edx; \
-    cpuid(&eax, &ecx, &ecx, &edx, NULL); \
-    (edx & (1U << 23)) != 0; \
-})
-
-#define cpu_has_sse ({ \
-    unsigned int eax = 1, ecx = 0, edx; \
-    cpuid(&eax, &ecx, &ecx, &edx, NULL); \
-    (edx & (1U << 25)) != 0; \
-})
-
-#define cpu_has_sse2 ({ \
-    unsigned int eax = 1, ecx = 0, edx; \
-    cpuid(&eax, &ecx, &ecx, &edx, NULL); \
-    (edx & (1U << 26)) != 0; \
-})
-
-#define cpu_has_xsave ({ \
-    unsigned int eax = 1, ecx = 0; \
-    cpuid(&eax, &eax, &ecx, &eax, NULL); \
-    /* Intentionally checking OSXSAVE here. */ \
-    (ecx & (1U << 27)) != 0; \
-})
-
-static inline uint64_t xgetbv(uint32_t xcr)
-{
-    uint32_t lo, hi;
-
-    asm ( ".byte 0x0f, 0x01, 0xd0" : "=a" (lo), "=d" (hi) : "c" (xcr) );
-
-    return ((uint64_t)hi << 32) | lo;
-}
-
-#define cpu_has_avx ({ \
-    unsigned int eax = 1, ecx = 0; \
-    cpuid(&eax, &eax, &ecx, &eax, NULL); \
-    if ( !(ecx & (1U << 27)) || ((xgetbv(0) & 6) != 6) ) \
-        ecx = 0; \
-    (ecx & (1U << 28)) != 0; \
-})
-
-#define cpu_has_avx2 ({ \
-    unsigned int eax = 1, ebx, ecx = 0; \
-    cpuid(&eax, &ebx, &ecx, &eax, NULL); \
-    if ( !(ecx & (1U << 27)) || ((xgetbv(0) & 6) != 6) ) \
-        ebx = 0; \
-    else { \
-        eax = 7, ecx = 0; \
-        cpuid(&eax, &ebx, &ecx, &eax, NULL); \
-    } \
-    (ebx & (1U << 5)) != 0; \
-})
-
-static int read_cr(
-    unsigned int reg,
-    unsigned long *val,
-    struct x86_emulate_ctxt *ctxt)
-{
-    /* Fake just enough state for the emulator's _get_fpu() to be happy. */
-    switch ( reg )
-    {
-    case 0:
-        *val = 0x00000001; /* PE */
-        return X86EMUL_OKAY;
-
-    case 4:
-        /* OSFXSR, OSXMMEXCPT, and maybe OSXSAVE */
-        *val = 0x00000600 | (cpu_has_xsave ? 0x00040000 : 0);
-        return X86EMUL_OKAY;
-    }
-
-    return X86EMUL_UNHANDLEABLE;
-}
-
 int get_fpu(
     void (*exception_callback)(void *, struct cpu_user_regs *),
     void *exception_callback_arg,
@@ -221,8 +122,8 @@ static struct x86_emulate_ops emulops = {
     .insn_fetch = fetch,
     .write      = write,
     .cmpxchg    = cmpxchg,
-    .cpuid      = cpuid,
-    .read_cr    = read_cr,
+    .cpuid      = emul_test_cpuid,
+    .read_cr    = emul_test_read_cr,
     .get_fpu    = get_fpu,
 };
 
diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index 963dd71..8b70580 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -38,4 +38,43 @@ bool emul_test_make_stack_executable(void)
                     MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC) == 0;
 }
 
+int emul_test_cpuid(
+    unsigned int *eax,
+    unsigned int *ebx,
+    unsigned int *ecx,
+    unsigned int *edx,
+    struct x86_emulate_ctxt *ctxt)
+{
+    unsigned int leaf = *eax;
+
+    asm ("cpuid" : "+a" (*eax), "+c" (*ecx), "=d" (*edx), "=b" (*ebx));
+
+    /* The emulator doesn't itself use MOVBE, so we can always run the test. */
+    if ( leaf == 1 )
+        *ecx |= 1U << 22;
+
+    return X86EMUL_OKAY;
+}
+
+int emul_test_read_cr(
+    unsigned int reg,
+    unsigned long *val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    /* Fake just enough state for the emulator's _get_fpu() to be happy. */
+    switch ( reg )
+    {
+    case 0:
+        *val = 0x00000001; /* PE */
+        return X86EMUL_OKAY;
+
+    case 4:
+        /* OSFXSR, OSXMMEXCPT, and maybe OSXSAVE */
+        *val = 0x00000600 | (cpu_has_xsave ? 0x00040000 : 0);
+        return X86EMUL_OKAY;
+    }
+
+    return X86EMUL_UNHANDLEABLE;
+}
+
 #include "x86_emulate/x86_emulate.c"
diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h
index a9b874c..4cc3f72 100644
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -37,3 +37,75 @@
 bool emul_test_make_stack_executable(void);
 
 #include "x86_emulate/x86_emulate.h"
+
+static inline uint64_t xgetbv(uint32_t xcr)
+{
+    uint32_t lo, hi;
+
+    asm ( ".byte 0x0f, 0x01, 0xd0" : "=a" (lo), "=d" (hi) : "c" (xcr) );
+
+    return ((uint64_t)hi << 32) | lo;
+}
+
+#define cache_line_size() ({		     \
+    unsigned int eax = 1, ebx, ecx = 0, edx; \
+    emul_test_cpuid(&eax, &ebx, &ecx, &edx, NULL); \
+    edx & (1U << 19) ? (ebx >> 5) & 0x7f8 : 0; \
+})
+
+#define cpu_has_mmx ({ \
+    unsigned int eax = 1, ecx = 0, edx; \
+    emul_test_cpuid(&eax, &ecx, &ecx, &edx, NULL); \
+    (edx & (1U << 23)) != 0; \
+})
+
+#define cpu_has_sse ({ \
+    unsigned int eax = 1, ecx = 0, edx; \
+    emul_test_cpuid(&eax, &ecx, &ecx, &edx, NULL); \
+    (edx & (1U << 25)) != 0; \
+})
+
+#define cpu_has_sse2 ({ \
+    unsigned int eax = 1, ecx = 0, edx; \
+    emul_test_cpuid(&eax, &ecx, &ecx, &edx, NULL); \
+    (edx & (1U << 26)) != 0; \
+})
+
+#define cpu_has_xsave ({ \
+    unsigned int eax = 1, ecx = 0; \
+    emul_test_cpuid(&eax, &eax, &ecx, &eax, NULL); \
+    /* Intentionally checking OSXSAVE here. */ \
+    (ecx & (1U << 27)) != 0; \
+})
+
+#define cpu_has_avx ({ \
+    unsigned int eax = 1, ecx = 0; \
+    emul_test_cpuid(&eax, &eax, &ecx, &eax, NULL); \
+    if ( !(ecx & (1U << 27)) || ((xgetbv(0) & 6) != 6) ) \
+        ecx = 0; \
+    (ecx & (1U << 28)) != 0; \
+})
+
+#define cpu_has_avx2 ({ \
+    unsigned int eax = 1, ebx, ecx = 0; \
+    emul_test_cpuid(&eax, &ebx, &ecx, &eax, NULL); \
+    if ( !(ecx & (1U << 27)) || ((xgetbv(0) & 6) != 6) ) \
+        ebx = 0; \
+    else { \
+        eax = 7, ecx = 0; \
+        emul_test_cpuid(&eax, &ebx, &ecx, &eax, NULL); \
+    } \
+    (ebx & (1U << 5)) != 0; \
+})
+
+int emul_test_cpuid(
+    unsigned int *eax,
+    unsigned int *ebx,
+    unsigned int *ecx,
+    unsigned int *edx,
+    struct x86_emulate_ctxt *ctxt);
+
+int emul_test_read_cr(
+    unsigned int reg,
+    unsigned long *val,
+    struct x86_emulate_ctxt *ctxt);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 4/7] x86emul/test: factor out emul_test_get_fpu

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/tests/x86_emulator/test_x86_emulator.c | 27 +--------------------------
 tools/tests/x86_emulator/x86_emulate.c       | 25 +++++++++++++++++++++++++
 tools/tests/x86_emulator/x86_emulate.h       |  6 ++++++
 3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index e40f0ea..04b8ca6 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -92,31 +92,6 @@ static int cmpxchg(
     return X86EMUL_OKAY;
 }
 
-int get_fpu(
-    void (*exception_callback)(void *, struct cpu_user_regs *),
-    void *exception_callback_arg,
-    enum x86_emulate_fpu_type type,
-    struct x86_emulate_ctxt *ctxt)
-{
-    switch ( type )
-    {
-    case X86EMUL_FPU_fpu:
-        break;
-    case X86EMUL_FPU_mmx:
-        if ( cpu_has_mmx )
-            break;
-    case X86EMUL_FPU_xmm:
-        if ( cpu_has_sse )
-            break;
-    case X86EMUL_FPU_ymm:
-        if ( cpu_has_avx )
-            break;
-    default:
-        return X86EMUL_UNHANDLEABLE;
-    }
-    return X86EMUL_OKAY;
-}
-
 static struct x86_emulate_ops emulops = {
     .read       = read,
     .insn_fetch = fetch,
@@ -124,7 +99,7 @@ static struct x86_emulate_ops emulops = {
     .cmpxchg    = cmpxchg,
     .cpuid      = emul_test_cpuid,
     .read_cr    = emul_test_read_cr,
-    .get_fpu    = get_fpu,
+    .get_fpu    = emul_test_get_fpu,
 };
 
 int main(int argc, char **argv)
diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index 8b70580..a666a32 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -77,4 +77,29 @@ int emul_test_read_cr(
     return X86EMUL_UNHANDLEABLE;
 }
 
+int emul_test_get_fpu(
+    void (*exception_callback)(void *, struct cpu_user_regs *),
+    void *exception_callback_arg,
+    enum x86_emulate_fpu_type type,
+    struct x86_emulate_ctxt *ctxt)
+{
+    switch ( type )
+    {
+    case X86EMUL_FPU_fpu:
+        break;
+    case X86EMUL_FPU_mmx:
+        if ( cpu_has_mmx )
+            break;
+    case X86EMUL_FPU_xmm:
+        if ( cpu_has_sse )
+            break;
+    case X86EMUL_FPU_ymm:
+        if ( cpu_has_avx )
+            break;
+    default:
+        return X86EMUL_UNHANDLEABLE;
+    }
+    return X86EMUL_OKAY;
+}
+
 #include "x86_emulate/x86_emulate.c"
diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h
index 4cc3f72..b4d1555 100644
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -109,3 +109,9 @@ int emul_test_read_cr(
     unsigned int reg,
     unsigned long *val,
     struct x86_emulate_ctxt *ctxt);
+
+int emul_test_get_fpu(
+    void (*exception_callback)(void *, struct cpu_user_regs *),
+    void *exception_callback_arg,
+    enum x86_emulate_fpu_type type,
+    struct x86_emulate_ctxt *ctxt);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Wei Liu]

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@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

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
+
+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..759f066
--- /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;
+static unsigned int data_max;
+
+static int data_read(const char *why, void *dst, unsigned int bytes)
+{
+    unsigned 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 int)*(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 = &regs,
+            .addr_size = 8 * sizeof(void *),
+            .sp_size = 8 * sizeof(void *),
+        };
+
+    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 */
+    memset(data, 0, sizeof(data));
+    data_index = 0;
+    data_max = 0;
+
+    nr = size < sizeof(regs) ? size : sizeof(regs);
+
+    memcpy(&regs, p, nr);
+    p += sizeof(regs);
+    nr += sizeof(regs);
+
+    if ( nr <= size )
+    {
+        memcpy(data, p, size - nr);
+        data_max = size - nr;
+    }
+
+    ctxt.force_writeback = 0;
+
+    /* Zero 'private' entries */
+    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@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 6/7] tools: hook up fuzz directory

[Wei Liu]

This will make all fuzzing targets get build every time tools directory
is built. This serves as basic regression test.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/Makefile      |  1 +
 tools/fuzz/Makefile | 11 +++++++++++
 2 files changed, 12 insertions(+)
 create mode 100644 tools/fuzz/Makefile

diff --git a/tools/Makefile b/tools/Makefile
index 71515b4..77e0723 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -6,6 +6,7 @@ SUBDIRS-y += include
 SUBDIRS-y += libs
 SUBDIRS-y += libxc
 SUBDIRS-y += flask
+SUBDIRS-y += fuzz
 SUBDIRS-y += xenstore
 SUBDIRS-y += misc
 SUBDIRS-y += examples
diff --git a/tools/fuzz/Makefile b/tools/fuzz/Makefile
new file mode 100644
index 0000000..ce00b82
--- /dev/null
+++ b/tools/fuzz/Makefile
@@ -0,0 +1,11 @@
+XEN_ROOT = $(CURDIR)/../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+SUBDIRS-y :=
+SUBDIRS-y += libelf
+SUBDIRS-y += x86_instruction_emulator
+
+.PHONY: all clean distclean
+all clean distclean: %: subdirs-%
+
+install:
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 7/7] tools/fuzz: add README

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/fuzz/README | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 tools/fuzz/README

diff --git a/tools/fuzz/README b/tools/fuzz/README
new file mode 100644
index 0000000..cf47bf6
--- /dev/null
+++ b/tools/fuzz/README
@@ -0,0 +1,39 @@
+# OVERVIEW
+
+This directory provides fuzzing targets to be run inside Google
+oss-fuzz infrastructure.
+
+See also https://github.com/google/oss-fuzz.
+
+# HOW IT WORKS
+
+We need to provide the source code and the rune to produce objects or
+archives (artefacts) from source code. These items ideally should live
+inside xen.git so that they can be kept up to date.
+
+The artefacts contain all the code we wish to fuzz and a function
+called LLVMFuzzerTestOneInput. LLVMFuzzerTestOneInput is the entry
+point to the code we wish to fuzz. Note that we don't produce
+executable programs because we don't have libFuzzEngine
+locally. libFuzzEngine is maintained by oss-fuzz.
+
+We also provide build script to oss-fuzz. The build script will
+inherit the correct compiler settings and be run in a pre-setup
+environment, which has libFuzzEngine installed. The build script is
+responsible for calling the correct Xen build rune to produce the
+artefacts, then link them against libFuzzEngine to produce
+executables, which will run in oss-fuzz infrastructure.
+
+Please refer to official oss-fuzz documents for the most up-to-date
+descriptions for all moving parts.
+
+# HOW TO IMPROVE THE FUZZING TARGETS
+
+Feel free to modify each fuzzing targets at will. Make sure they build
+by invoking make as you would build tools.
+
+To actually test the new code, you would need to run the target in
+standalone mode, please refer to oss-fuzz documents on how to do that.
+
+It is highly recommended that you run the new target for a while to
+weed out error in plumbing code to avoid false positives.
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 1/7] tools/fuzz: introduce libelf target

[Jan Beulich]

>>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> wrote:
> Source code and Makefile to fuzz libelf in Google's oss-fuzz
> infrastructure.
> 
> Introduce FUZZ_NO_LIBXC in libelf-private.h. That macro will be set when
> compiling libelf fuzzer target because libxc is not required in libelf
> fuzzing.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 3/7] x86emul/test: factor out emul_test_{read_cr, cpuid}

[Jan Beulich]

>>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> wrote:
> While at it, move xgetbv, all cpu_has_* and cache_line_size macros to
> x86_emulate.h.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one further cosmetic request:

> --- a/tools/tests/x86_emulator/x86_emulate.c
> +++ b/tools/tests/x86_emulator/x86_emulate.c
> @@ -38,4 +38,43 @@ bool emul_test_make_stack_executable(void)
>                      MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC) == 0;
>  }
>  
> +int emul_test_cpuid(
> +    unsigned int *eax,
> +    unsigned int *ebx,
> +    unsigned int *ecx,
> +    unsigned int *edx,
> +    struct x86_emulate_ctxt *ctxt)
> +{
> +    unsigned int leaf = *eax;
> +
> +    asm ("cpuid" : "+a" (*eax), "+c" (*ecx), "=d" (*edx), "=b" (*ebx));
> +
> +    /* The emulator doesn't itself use MOVBE, so we can always run the test. */
> +    if ( leaf == 1 )
> +        *ecx |= 1U << 22;

The comment here wants some adjustment: "the test" is no longer
applicable. Perhaps "respective tests"?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 4/7] x86emul/test: factor out emul_test_get_fpu

[Jan Beulich]

>>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 3/7] x86emul/test: factor out emul_test_{read_cr, cpuid}

[Wei Liu]

On Mon, Dec 12, 2016 at 02:45:45AM -0700, Jan Beulich wrote:
> >>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> wrote:
> > While at it, move xgetbv, all cpu_has_* and cache_line_size macros to
> > x86_emulate.h.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with one further cosmetic request:
> 
> > --- a/tools/tests/x86_emulator/x86_emulate.c
> > +++ b/tools/tests/x86_emulator/x86_emulate.c
> > @@ -38,4 +38,43 @@ bool emul_test_make_stack_executable(void)
> >                      MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC) == 0;
> >  }
> >  
> > +int emul_test_cpuid(
> > +    unsigned int *eax,
> > +    unsigned int *ebx,
> > +    unsigned int *ecx,
> > +    unsigned int *edx,
> > +    struct x86_emulate_ctxt *ctxt)
> > +{
> > +    unsigned int leaf = *eax;
> > +
> > +    asm ("cpuid" : "+a" (*eax), "+c" (*ecx), "=d" (*edx), "=b" (*ebx));
> > +
> > +    /* The emulator doesn't itself use MOVBE, so we can always run the test. */
> > +    if ( leaf == 1 )
> > +        *ecx |= 1U << 22;
> 
> The comment here wants some adjustment: "the test" is no longer
> applicable. Perhaps "respective tests"?
> 

Sure. I will make the change in my branch.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Jan Beulich]

>>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> 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@citrix.com>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> 
> 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]

?

> --- /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.

> +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.

> +    if ( data_index + bytes > data_max )
> +        return X86EMUL_EXCEPTION;
> +
> +    memcpy(dst,  data+data_index, bytes);

Blanks around binary operators please (more further down).

> +    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?

> +int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
> +{
> +    bool stack_exec;
> +    struct cpu_user_regs regs = {};
> +    struct x86_emulate_ctxt ctxt =
> +        {
> +            .regs = &regs,
> +            .addr_size = 8 * sizeof(void *),
> +            .sp_size = 8 * sizeof(void *),
> +        };
> +

Stray blank line. The indentation of the initializer above also looks
a little unusual.

> +    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"?

> +    memset(data, 0, sizeof(data));
> +    data_index = 0;
> +    data_max = 0;
> +
> +    nr = size < sizeof(regs) ? size : sizeof(regs);
> +
> +    memcpy(&regs, 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.

> +    {
> +        memcpy(data, p, size - nr);
> +        data_max = size - nr;
> +    }
> +
> +    ctxt.force_writeback = 0;

false

> +    /* Zero 'private' entries */

s/entries/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;

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).

> +    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;
> +    }

Pointless braces.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Wei Liu]

On Mon, Dec 12, 2016 at 02:58:39AM -0700, Jan Beulich wrote:
> >>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> 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@citrix.com>
> > Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > 
> > 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 = &regs,
> > +            .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(&regs, 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@citrix.com>
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@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

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 = &regs,
+        .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(&regs, 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@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Jan Beulich]

>>> On 12.12.16 at 12:19, <wei.liu2@citrix.com> wrote:
> On Mon, Dec 12, 2016 at 02:58:39AM -0700, Jan Beulich wrote:
>> >>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> wrote:
>> > +    /* Reset all global states */
>> 
>> DYM "state"?
> 
> I mean "states". There are three states we need to reset.

Hmm, to me as a non-native speaker it feels wrong to use plural
here (just like for e.g. milk), but maybe I'm wrong.

> From: Wei Liu <wei.liu2@citrix.com>
> 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@citrix.com>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> 
> v4:
> 1. more coding style fixes and bug fixes
> 2. only do 64 bit build

Reviewed-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Wei Liu]

On Mon, Dec 12, 2016 at 04:30:30AM -0700, Jan Beulich wrote:
> >>> On 12.12.16 at 12:19, <wei.liu2@citrix.com> wrote:
> > On Mon, Dec 12, 2016 at 02:58:39AM -0700, Jan Beulich wrote:
> >> >>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> wrote:
> >> > +    /* Reset all global states */
> >> 
> >> DYM "state"?
> > 
> > I mean "states". There are three states we need to reset.
> 
> Hmm, to me as a non-native speaker it feels wrong to use plural
> here (just like for e.g. milk), but maybe I'm wrong.
> 

"State" is countable when used to represent the condition of
somebody/something.  Various other meanings of "state" don't apply here.

Sources:

https://www.oxfordlearnersdictionaries.com/definition/english/state_1
http://www.ldoceonline.com/dictionary/state

> > From: Wei Liu <wei.liu2@citrix.com>
> > 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@citrix.com>
> > Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > 
> > v4:
> > 1. more coding style fixes and bug fixes
> > 2. only do 64 bit build
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 

Thanks.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Wei Liu]

On Mon, Dec 12, 2016 at 02:58:39AM -0700, Jan Beulich wrote:
> >>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> 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@citrix.com>
> > Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > 
> > 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]
> 
> ?
> 

Ah, I thought this would work, but it doesn't. And I forgot to remove
the old links when testing this.

Now in a clean build:

make[2]: *** No rule to make target 'x86_emulate.[ch]', needed by 'x86_emulate.o'.  Stop.

So I guess I will stick with what I had before.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target"):
> On Mon, Dec 12, 2016 at 04:30:30AM -0700, Jan Beulich wrote:
>>> On 12.12.16 at 12:19, <wei.liu2@citrix.com> wrote:
> > > I mean "states". There are three states we need to reset.
> > 
> > Hmm, to me as a non-native speaker it feels wrong to use plural
> > here (just like for e.g. milk), but maybe I'm wrong.
> 
> "State" is countable when used to represent the condition of
> somebody/something.  Various other meanings of "state" don't apply here.

I think "three states" can only be used to mean three alternative
possible values of the same variable.  It cannot be used to mean three
separate variables.

So "a state" is one of the possible values.  You can also say
something like "the system is in a broken state", but again, this
refers to a specific state amongst all the possible different states.

"Reset all states" is really odd and could almost never be right.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Wei Liu]

On Mon, Dec 12, 2016 at 05:59:40PM +0000, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target"):
> > On Mon, Dec 12, 2016 at 04:30:30AM -0700, Jan Beulich wrote:
> >>> On 12.12.16 at 12:19, <wei.liu2@citrix.com> wrote:
> > > > I mean "states". There are three states we need to reset.
> > > 
> > > Hmm, to me as a non-native speaker it feels wrong to use plural
> > > here (just like for e.g. milk), but maybe I'm wrong.
> > 
> > "State" is countable when used to represent the condition of
> > somebody/something.  Various other meanings of "state" don't apply here.
> 
> I think "three states" can only be used to mean three alternative
> possible values of the same variable.  It cannot be used to mean three
> separate variables.
> 
> So "a state" is one of the possible values.  You can also say
> something like "the system is in a broken state", but again, this
> refers to a specific state amongst all the possible different states.
> 
> "Reset all states" is really odd and could almost never be right.
> 

David suggested "reset all state variables". That's what I have now.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[Jan Beulich]

>>> On 12.12.16 at 18:51, <wei.liu2@citrix.com> wrote:
> On Mon, Dec 12, 2016 at 02:58:39AM -0700, Jan Beulich wrote:
>> >>> On 12.12.16 at 10:28, <wei.liu2@citrix.com> 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@citrix.com>
>> > Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> > ---
>> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> > Cc: Jan Beulich <jbeulich@suse.com>
>> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> > Cc: Stefano Stabellini <sstabellini@kernel.org>
>> > Cc: Tim Deegan <tim@xen.org>
>> > Cc: Wei Liu <wei.liu2@citrix.com>
>> > 
>> > 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]
>> 
>> ?
>> 
> 
> Ah, I thought this would work, but it doesn't. And I forgot to remove
> the old links when testing this.
> 
> Now in a clean build:
> 
> make[2]: *** No rule to make target 'x86_emulate.[ch]', needed by 
> 'x86_emulate.o'.  Stop.
> 
> So I guess I will stick with what I had before.

Oh, right - I'm sorry for misleading you. Wildcards here would work
only if the files existed, but there are rules to put them in place. No
need to drop my R-b, btw.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 5/7] tools/fuzz: introduce x86 instruction emulator target

[George Dunlap]

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

On Mon, Dec 12, 2016 at 5:28 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> Instruction emulator fuzzing code is from code previous written by
> Andrew and George. Adapted to llvm fuzzer and hook up the build system.

Thanks for doing this, Wei.

The version you have is quite an early iteration -- attached is a
patch containing the most recent version if you feel like updating it.

Long-term, it seems like being able to make the same code compile for
AFL and for the oss-fuzz would be a good idea.

I'll probably take a look at it when I get back in January if you
haven't done it by then.

Peace,
 -George

[-- Attachment #2: 0001-AFL-wrapper-for-the-x86-emulator-test-harness.patch --]
[-- Type: application/force-download, Size: 41968 bytes --]

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel