Message ID | f524a6da-75be-542e-bf28-ef5ad056053e@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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>
>>> 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
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