diff mbox

[v2,for-4.9,5/7] tools/insn-fuzz: Correct hook prototypes, and assert() appropriate segments

Message ID 1491414813-30003-6-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper April 5, 2017, 5:53 p.m. UTC 
The correct prototypes for the hooks are to use enum x86_segment rather than
unsigned int.  It is implementation specific as to whether this compiles.

assert() that the emulator never passes an inappropriate segment.  The only
hook which may legitimately be passed x86_seg_none is invlpg().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: George Dunlap <george.dunlap@eu.citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/fuzz/x86_instruction_emulator/fuzz-emul.c | 45 ++++++++++++++++++++-----
 tools/tests/x86_emulator/test_x86_emulator.c    |  8 ++---
 2 files changed, 41 insertions(+), 12 deletions(-)

Comments

Jan Beulich April 6, 2017, 9:28 a.m. UTC | #1 
>>> On 05.04.17 at 19:53, <andrew.cooper3@citrix.com> wrote:
> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> @@ -117,12 +117,15 @@ static int data_read(struct x86_emulate_ctxt *ctxt,
>  }
>  
>  static int fuzz_read(
> -    unsigned int seg,
> +    enum x86_segment seg,
>      unsigned long offset,
>      void *p_data,
>      unsigned int bytes,
>      struct x86_emulate_ctxt *ctxt)
>  {
> +    /* Reads expected for all user and system segments. */
> +    assert((unsigned int)seg < x86_seg_none);

Would this perhaps be more clear as
"is_user_segment() || is_system_segment()"?

> @@ -274,6 +302,9 @@ static int fuzz_invlpg(
>      unsigned long offset,
>      struct x86_emulate_ctxt *ctxt)
>  {
> +    /* invlpg(), unlike all other hooks, may be called with x86_seg_none. */
> +    assert((unsigned int)seg <= x86_seg_none);

But no system segment, so rather
"is_user_segment() || seg == none"?

> @@ -300,8 +331,7 @@ static int fuzz_read_segment(
>      const struct fuzz_state *s = ctxt->data;
>      const struct fuzz_corpus *c = s->corpus;
>  
> -    if ( seg >= SEG_NUM )
> -        return X86EMUL_UNHANDLEABLE;
> +    assert((unsigned int)seg < x86_seg_none);

Same as for fuzz_read().

> @@ -317,8 +347,7 @@ static int fuzz_write_segment(
>      struct fuzz_corpus *c = s->corpus;
>      int rc;
>  
> -    if ( seg >= SEG_NUM )
> -        return X86EMUL_UNHANDLEABLE;
> +    assert((unsigned int)seg < x86_seg_none);

And here.

With at least the invlpg part taken care of
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox

Patch

diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
index a20212e..fedeb9f 100644
--- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
+++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
@@ -117,12 +117,15 @@  static int data_read(struct x86_emulate_ctxt *ctxt,
 }
 
 static int fuzz_read(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
+    /* Reads expected for all user and system segments. */
+    assert((unsigned int)seg < x86_seg_none);
+
     return data_read(ctxt, "read", p_data, bytes);
 }
 
@@ -136,12 +139,14 @@  static int fuzz_read_io(
 }
 
 static int fuzz_insn_fetch(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
+    assert(seg == x86_seg_cs);
+
     /*
      * Zero-length instruction fetches are made at the destination of jumps,
      * to perform segmentation checks.  No data needs returning.
@@ -211,6 +216,8 @@  static int fuzz_rep_ins(
     unsigned long *reps,
     struct x86_emulate_ctxt *ctxt)
 {
+    assert(dst_seg == x86_seg_es);
+
     return _fuzz_rep_read(ctxt, "rep_ins", reps);
 }
 
@@ -223,6 +230,9 @@  static int fuzz_rep_movs(
     unsigned long *reps,
     struct x86_emulate_ctxt *ctxt)
 {
+    assert(is_x86_user_segment(src_seg));
+    assert(dst_seg == x86_seg_es);
+
     return _fuzz_rep_read(ctxt, "rep_movs", reps);
 }
 
@@ -234,6 +244,8 @@  static int fuzz_rep_outs(
     unsigned long *reps,
     struct x86_emulate_ctxt *ctxt)
 {
+    assert(is_x86_user_segment(src_seg));
+
     return _fuzz_rep_write(ctxt, "rep_outs", reps);
 }
 
@@ -245,27 +257,43 @@  static int fuzz_rep_stos(
     unsigned long *reps,
     struct x86_emulate_ctxt *ctxt)
 {
+    /*
+     * STOS itself may only have an %es segment, but the stos() hook is reused
+     * for CLZERO.
+     */
+    assert(is_x86_user_segment(seg));
+
     return _fuzz_rep_write(ctxt, "rep_stos", reps);
 }
 
 static int fuzz_write(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
+    /* Writes not expected for any system segments. */
+    assert(is_x86_user_segment(seg));
+
     return maybe_fail(ctxt, "write", true);
 }
 
 static int fuzz_cmpxchg(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *old,
     void *new,
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
+    /*
+     * Cmpxchg expected for user segments, and setting accessed/busy bits in
+     * GDT/LDT enties, but not expected for any IDT or TR accesses.
+     */
+    assert(is_x86_user_segment(seg) ||
+           seg == x86_seg_gdtr || seg == x86_seg_ldtr);
+
     return maybe_fail(ctxt, "cmpxchg", true);
 }
 
@@ -274,6 +302,9 @@  static int fuzz_invlpg(
     unsigned long offset,
     struct x86_emulate_ctxt *ctxt)
 {
+    /* invlpg(), unlike all other hooks, may be called with x86_seg_none. */
+    assert((unsigned int)seg <= x86_seg_none);
+
     return maybe_fail(ctxt, "invlpg", false);
 }
 
@@ -300,8 +331,7 @@  static int fuzz_read_segment(
     const struct fuzz_state *s = ctxt->data;
     const struct fuzz_corpus *c = s->corpus;
 
-    if ( seg >= SEG_NUM )
-        return X86EMUL_UNHANDLEABLE;
+    assert((unsigned int)seg < x86_seg_none);
 
     *reg = c->segments[seg];
 
@@ -317,8 +347,7 @@  static int fuzz_write_segment(
     struct fuzz_corpus *c = s->corpus;
     int rc;
 
-    if ( seg >= SEG_NUM )
-        return X86EMUL_UNHANDLEABLE;
+    assert((unsigned int)seg < x86_seg_none);
 
     rc = maybe_fail(ctxt, "write_segment", true);
 
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index efeb175..1992c3f 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -155,7 +155,7 @@  static const struct {
 static unsigned int bytes_read;
 
 static int read(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
@@ -210,7 +210,7 @@  static int read(
 }
 
 static int fetch(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
@@ -224,7 +224,7 @@  static int fetch(
 }
 
 static int write(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
@@ -240,7 +240,7 @@  static int write(
 }
 
 static int cmpxchg(
-    unsigned int seg,
+    enum x86_segment seg,
     unsigned long offset,
     void *old,
     void *new,