diff mbox

[v3,01/12] fuzz/x86_emulate: Clear errors after each iteration

Message ID f524a6da-75be-542e-bf28-ef5ad056053e@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

George Dunlap Oct. 10, 2017, 4:47 p.m. UTC
On 10/10/2017 05:20 PM, George Dunlap wrote:
> Once feof() returns true for a stream, it will continue to return true
> for that stream until clearerr() is called (or the stream is closed
> and re-opened).
> 
> In llvm-clang-fast-mode, the same file descriptor is used for each
> iteration of the loop, meaning that the "Input too large" check was
> broken -- feof() would return true even if the fread() hadn't hit the
> end of the file.  The result is that AFL generates testcases of
> arbitrary size.
> 
> Fix this by fseek'ing to the beginning of the file on every iteration;
> this resets the EOF marker and other state.
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> Changes in v3:
> - Fix the issue in the official sanctioned way

Hmm, seems v2 of this patch was checked in; review had flagged up that
"clearerr()" was too big of a hammer.

Attached is a revised v1/12 patch that fixes this.

 -George

Comments

Andrew Cooper Oct. 10, 2017, 4:47 p.m. UTC | #1
On 10/10/17 17:47, George Dunlap wrote:
> On 10/10/2017 05:20 PM, George Dunlap wrote:
>> Once feof() returns true for a stream, it will continue to return true
>> for that stream until clearerr() is called (or the stream is closed
>> and re-opened).
>>
>> In llvm-clang-fast-mode, the same file descriptor is used for each
>> iteration of the loop, meaning that the "Input too large" check was
>> broken -- feof() would return true even if the fread() hadn't hit the
>> end of the file.  The result is that AFL generates testcases of
>> arbitrary size.
>>
>> Fix this by fseek'ing to the beginning of the file on every iteration;
>> this resets the EOF marker and other state.
>>
>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>> ---
>> Changes in v3:
>> - Fix the issue in the official sanctioned way
> Hmm, seems v2 of this patch was checked in; review had flagged up that
> "clearerr()" was too big of a hammer.
>
> Attached is a revised v1/12 patch that fixes this.
>
>  -George

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich Oct. 11, 2017, 8:59 a.m. UTC | #2
>>> On 10.10.17 at 18:47, <george.dunlap@citrix.com> wrote:
> On 10/10/2017 05:20 PM, George Dunlap wrote:
>> Once feof() returns true for a stream, it will continue to return true
>> for that stream until clearerr() is called (or the stream is closed
>> and re-opened).
>> 
>> In llvm-clang-fast-mode, the same file descriptor is used for each
>> iteration of the loop, meaning that the "Input too large" check was
>> broken -- feof() would return true even if the fread() hadn't hit the
>> end of the file.  The result is that AFL generates testcases of
>> arbitrary size.
>> 
>> Fix this by fseek'ing to the beginning of the file on every iteration;
>> this resets the EOF marker and other state.
>> 
>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>> ---
>> Changes in v3:
>> - Fix the issue in the official sanctioned way
> 
> Hmm, seems v2 of this patch was checked in; review had flagged up that
> "clearerr()" was too big of a hammer.

Oh, I'm sorry for having overlooked that feedback.

Jan
diff mbox

Patch

From d07b2d68085957bf3d7a2567dce9c4f031fb5966 Mon Sep 17 00:00:00 2001
From: George Dunlap <george.dunlap@citrix.com>
Date: Wed, 4 Oct 2017 17:09:10 +0100
Subject: [PATCH] fuzz/x86_emulate: Clear errors in the officially sanctioned
 way

Commit 849a1f10c9 was checked in in appropriately; review flagged up
that clearerr() was too big a hammer, as it would clear both the EOF
flag and stream errors.

Stream errors shouldn't be cleared; we only want the EOF and other
stream-related state cleared.  To do this, it is sufficient to fseek()
to zero.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
This is a candidate for backport to 4.9.

CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 tools/fuzz/x86_instruction_emulator/afl-harness.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/afl-harness.c b/tools/fuzz/x86_instruction_emulator/afl-harness.c
index b4d15451b5..31ae1daef1 100644
--- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
+++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
@@ -77,6 +77,17 @@  int main(int argc, char **argv)
                 exit(-1);
             }
         }
+#ifdef __AFL_HAVE_MANUAL_CONTROL
+        else
+        {
+            /* 
+             * This will ensure we're dealing with a clean stream
+             * state after the afl-fuzz process messes with the open
+             * file handle.
+             */
+            fseek(fp, 0, SEEK_SET);
+        }
+#endif
 
         size = fread(input, 1, INPUT_SIZE, fp);
 
@@ -97,8 +108,6 @@  int main(int argc, char **argv)
             fclose(fp);
             fp = NULL;
         }
-        else
-            clearerr(fp);
 
         LLVMFuzzerTestOneInput(input, size);
     }
-- 
2.14.2