diff mbox

[01/14] fuzz/x86_emulate: Remove redundant AFL hook

Message ID 20170825164343.29015-1-george.dunlap@citrix.com
State New, archived
Headers show

Commit Message

George Dunlap Aug. 25, 2017, 4:43 p.m. UTC
You don't need __AFL_INIT if you have __AFL_LOOP.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
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 | 2 --
 1 file changed, 2 deletions(-)

Comments

Andrew Cooper Aug. 25, 2017, 5:37 p.m. UTC | #1
On 25/08/17 17:43, George Dunlap wrote:
> You don't need __AFL_INIT if you have __AFL_LOOP.
>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

Really?  Is that covered in any documentation?

I got the contrary impression from whichever version of AFL I was using
when I put this in, and a quick look over the afl-fuzz source doesn't
appear to equate them in any way.

~Andrew

> ---
> 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 | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/tools/fuzz/x86_instruction_emulator/afl-harness.c b/tools/fuzz/x86_instruction_emulator/afl-harness.c
> index 154869336a..1a79ff228e 100644
> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
> @@ -63,8 +63,6 @@ int main(int argc, char **argv)
>          exit(-1);
>  
>  #ifdef __AFL_HAVE_MANUAL_CONTROL
> -    __AFL_INIT();
> -
>      while ( __AFL_LOOP(1000) )
>  #endif
>      {
George Dunlap Aug. 28, 2017, 10:34 a.m. UTC | #2
On 08/25/2017 06:37 PM, Andrew Cooper wrote:
> On 25/08/17 17:43, George Dunlap wrote:
>> You don't need __AFL_INIT if you have __AFL_LOOP.
>>
>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> 
> Really?  Is that covered in any documentation?
> 
> I got the contrary impression from whichever version of AFL I was using
> when I put this in, and a quick look over the afl-fuzz source doesn't
> appear to equate them in any way.

The documentation does seem a bit hazy on the subject.  However:

1. It clear from the documentation [1] that both of them work *only* in
llvm mode (i.e., when compiled with afl-clang-fast).  In particular the
last paragraph of section 4: "afl-gcc or afl-clang will
*not* generate a deferred-initialization binary".

2. The documentation does seem to speak of them as separate 'modes'
(Section 5, "Note that as with the previous mode, ...")

3. Empirically speaking, persistent mode works fine with __AFL_LOOP()
and no __AFL_INIT() (for me anyway).

 -George

[1] https://github.com/mirrorer/afl/tree/master/llvm_mode

> 
> ~Andrew
> 
>> ---
>> 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 | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/tools/fuzz/x86_instruction_emulator/afl-harness.c b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> index 154869336a..1a79ff228e 100644
>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> @@ -63,8 +63,6 @@ int main(int argc, char **argv)
>>          exit(-1);
>>  
>>  #ifdef __AFL_HAVE_MANUAL_CONTROL
>> -    __AFL_INIT();
>> -
>>      while ( __AFL_LOOP(1000) )
>>  #endif
>>      {
>
George Dunlap Sept. 14, 2017, 3:26 p.m. UTC | #3
Ping?

I realize this isn't a  major feature but it would be nice to get it
in for 4.10.

 -George

On Mon, Aug 28, 2017 at 11:34 AM, George Dunlap
<george.dunlap@citrix.com> wrote:
> On 08/25/2017 06:37 PM, Andrew Cooper wrote:
>> On 25/08/17 17:43, George Dunlap wrote:
>>> You don't need __AFL_INIT if you have __AFL_LOOP.
>>>
>>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>>
>> Really?  Is that covered in any documentation?
>>
>> I got the contrary impression from whichever version of AFL I was using
>> when I put this in, and a quick look over the afl-fuzz source doesn't
>> appear to equate them in any way.
>
> The documentation does seem a bit hazy on the subject.  However:
>
> 1. It clear from the documentation [1] that both of them work *only* in
> llvm mode (i.e., when compiled with afl-clang-fast).  In particular the
> last paragraph of section 4: "afl-gcc or afl-clang will
> *not* generate a deferred-initialization binary".
>
> 2. The documentation does seem to speak of them as separate 'modes'
> (Section 5, "Note that as with the previous mode, ...")
>
> 3. Empirically speaking, persistent mode works fine with __AFL_LOOP()
> and no __AFL_INIT() (for me anyway).
>
>  -George
>
> [1] https://github.com/mirrorer/afl/tree/master/llvm_mode
>
>>
>> ~Andrew
>>
>>> ---
>>> 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 | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/tools/fuzz/x86_instruction_emulator/afl-harness.c b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>> index 154869336a..1a79ff228e 100644
>>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>> @@ -63,8 +63,6 @@ int main(int argc, char **argv)
>>>          exit(-1);
>>>
>>>  #ifdef __AFL_HAVE_MANUAL_CONTROL
>>> -    __AFL_INIT();
>>> -
>>>      while ( __AFL_LOOP(1000) )
>>>  #endif
>>>      {
>>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
George Dunlap Sept. 22, 2017, 3:47 p.m. UTC | #4
On Fri, Aug 25, 2017 at 6:37 PM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 25/08/17 17:43, George Dunlap wrote:
>> You don't need __AFL_INIT if you have __AFL_LOOP.
>>
>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>
> Really?  Is that covered in any documentation?
>
> I got the contrary impression from whichever version of AFL I was using
> when I put this in, and a quick look over the afl-fuzz source doesn't
> appear to equate them in any way.

Trying to answer the feof() question, I dug into llvm mode a bit more.
They are independent features but they can be used together: that is,
if you compile with afl-clang-fast, you always get a forkserver.  If
you specify the location with __AFL_INIT, that's where the forkserver
get started; otherwise, it's somewhere before main() is called.

When __AFL_LOOP(N) is present, it will execute the loop N times before
calling exit(); at which point, the forkserver will fork another
instance (either before main(), or at __AFL_INIT).

So you don't *need* __AFL_INIT; and the amount of time it will save
isn't much (the initialization every 1000 iterations), but I suppose
we might as well keep it.

 -George
Andrew Cooper Sept. 22, 2017, 4:09 p.m. UTC | #5
On 22/09/17 16:47, George Dunlap wrote:
> On Fri, Aug 25, 2017 at 6:37 PM, Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>> On 25/08/17 17:43, George Dunlap wrote:
>>> You don't need __AFL_INIT if you have __AFL_LOOP.
>>>
>>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>> Really?  Is that covered in any documentation?
>>
>> I got the contrary impression from whichever version of AFL I was using
>> when I put this in, and a quick look over the afl-fuzz source doesn't
>> appear to equate them in any way.
> Trying to answer the feof() question, I dug into llvm mode a bit more.
> They are independent features but they can be used together: that is,
> if you compile with afl-clang-fast, you always get a forkserver.  If
> you specify the location with __AFL_INIT, that's where the forkserver
> get started; otherwise, it's somewhere before main() is called.
>
> When __AFL_LOOP(N) is present, it will execute the loop N times before
> calling exit(); at which point, the forkserver will fork another
> instance (either before main(), or at __AFL_INIT).
>
> So you don't *need* __AFL_INIT; and the amount of time it will save
> isn't much (the initialization every 1000 iterations), but I suppose
> we might as well keep it.

The purpose of c/s 63092064eb was very deliberately to cause the 
mprotect() remapping the stack as executable to be not repeated.

~Andrew
diff mbox

Patch

diff --git a/tools/fuzz/x86_instruction_emulator/afl-harness.c b/tools/fuzz/x86_instruction_emulator/afl-harness.c
index 154869336a..1a79ff228e 100644
--- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
+++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
@@ -63,8 +63,6 @@  int main(int argc, char **argv)
         exit(-1);
 
 #ifdef __AFL_HAVE_MANUAL_CONTROL
-    __AFL_INIT();
-
     while ( __AFL_LOOP(1000) )
 #endif
     {