[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH VERY RFC 3/5] tools/fuzz: introduce x86 instruction emulator target



>>> On 08.12.16 at 14:54, <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.

With this, how much of the new code could be shared between
Google's fuzzer and AFL, for which George had put this together
originally afaik? Or are we now no longer planning on having an
AFL target?

> --- /dev/null
> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> @@ -0,0 +1,33 @@
> +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:
> +     [ -L x86_emulate.c ] || ln -sf 
> $(XEN_ROOT)/tools/tests/x86_emulator/x86_emulate.c
> +
> +x86_emulate.h:
> +     [ -L x86_emulate.h ] || ln -sf 
> $(XEN_ROOT)/tools/tests/x86_emulator/x86_emulate.h

I think these two could easily be a single (pattern) rule, with (slightly
unusually) the file extension being the stem.

> --- /dev/null
> +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
> @@ -0,0 +1,335 @@
> +#include <errno.h>
> +#include <limits.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <xen/xen.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <sys/mman.h>
> +
> +#define __packed __attribute__((packed))
> +#define ASSERT assert

This should not be needed anymore if you included ...

> +#include "x86_emulate/x86_emulate.h"

"x86_emulate.h" instead. And __packed, if its really needed,
should then also be added to that header.

> +/* EFLAGS bit definitions. */
> +#define EFLG_OF (1<<11)
> +#define EFLG_DF (1<<10)
> +#define EFLG_SF (1<<7)
> +#define EFLG_ZF (1<<6)
> +#define EFLG_AF (1<<4)
> +#define EFLG_PF (1<<2)
> +#define EFLG_CF (1<<0)

These appear to be unused.

> +static unsigned char data[4096];
> +static unsigned int data_index = 0;
> +static unsigned int data_max;
> +
> +int data_read(const char *why, void *dst, unsigned int bytes) {
> +

static? And the brace put on the otherwise stray blank line.

> +static int emul_read(

May I suggest to prefix all of these fuzz_ instead of emul_?

> +#define cpu_has_sse2 ({ \
> +    unsigned int eax = 1, ecx = 0, edx; \
> +    emul_cpuid(&eax, &ecx, &ecx, &edx, NULL); \
> +    (edx & (1U << 26)) != 0; \
> +})

This appears to be unused again.

> +#define cpu_has_avx2 ({ \
> +    unsigned int eax = 1, ebx, ecx = 0; \
> +    emul_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; \
> +})

Same here.

> +static int emul_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;
> +}

Looks suspiciously similar to existing code. We may want to share
such stuff, to avoid having to update it in multiple places.

> +int emul_get_fpu(

static?

> +struct x86_emulate_ops emulops = {

again

> +bool make_stack_executable(void) {
> +    unsigned long sp;
> +    bool stack_exec;
> +
> +    /*
> +     * Mark the entire stack executable so that the stub executions
> +     * don't fault
> +     */
> +#define MMAP_SZ 16384
> +
> +#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;
> +    if ( !stack_exec )
> +        printf("Warning: Stack could not be made executable (%d).\n", errno);
> +
> +    return stack_exec;
> +}

This too looks like it would want sharing.

Jan

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.