* [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
@ 2017-10-13 9:00 George Dunlap
2017-10-13 9:06 ` Jan Beulich
2017-10-17 13:39 ` Julien Grall
0 siblings, 2 replies; 9+ messages in thread
From: George Dunlap @ 2017-10-13 9:00 UTC (permalink / raw)
To: xen-devel
Cc: Wei Liu, Andrew Cooper, George Dunlap, Julien Grall, Jan Beulich,
Ian Jackson
Changeset XXXX introduced "batch mode" to afl-harness, which allowed
the handling of several inputs in sequence.
Unfortunately, it introduced a file pointer leak when the file was
larger than the maximum size. Restructure the code to always close fp
if we opened it.
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
Release exception justification:
- This is a bug fix. The problem is relatively minor, but the fix is relatively minor too.
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Julien Grall <julien.grall@arm.com>
---
tools/fuzz/x86_instruction_emulator/afl-harness.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/fuzz/x86_instruction_emulator/afl-harness.c b/tools/fuzz/x86_instruction_emulator/afl-harness.c
index d514468dd2..a2bae46d98 100644
--- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
+++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
@@ -99,13 +99,17 @@ int main(int argc, char **argv)
exit(-1);
}
- if ( !feof(fp) )
+ /* Only run the test if the input file was smaller than INPUT_SIZE */
+ if ( feof(fp) )
+ {
+ LLVMFuzzerTestOneInput(input, size);
+ }
+ else
{
printf("Input too large\n");
/* Don't exit if we're doing batch processing */
if ( max == 1 )
exit(-1);
- continue;
}
if ( fp != stdin )
@@ -113,8 +117,6 @@ int main(int argc, char **argv)
fclose(fp);
fp = NULL;
}
-
- LLVMFuzzerTestOneInput(input, size);
}
return 0;
--
2.14.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 9:00 [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak George Dunlap
@ 2017-10-13 9:06 ` Jan Beulich
2017-10-13 9:10 ` George Dunlap
2017-10-13 9:12 ` George Dunlap
2017-10-17 13:39 ` Julien Grall
1 sibling, 2 replies; 9+ messages in thread
From: Jan Beulich @ 2017-10-13 9:06 UTC (permalink / raw)
To: George Dunlap
Cc: Andrew Cooper, Julien Grall, Wei Liu, xen-devel, Ian Jackson
>>> On 13.10.17 at 11:00, <george.dunlap@citrix.com> wrote:
> Changeset XXXX introduced "batch mode" to afl-harness, which allowed
With (part of) the commit hash and the title inserted here and ...
> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
> @@ -99,13 +99,17 @@ int main(int argc, char **argv)
> exit(-1);
> }
>
> - if ( !feof(fp) )
> + /* Only run the test if the input file was smaller than INPUT_SIZE */
> + if ( feof(fp) )
> + {
> + LLVMFuzzerTestOneInput(input, size);
> + }
... ideally with the unnecessary braces dropped here
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 9:06 ` Jan Beulich
@ 2017-10-13 9:10 ` George Dunlap
2017-10-13 9:20 ` Jan Beulich
2017-10-13 9:12 ` George Dunlap
1 sibling, 1 reply; 9+ messages in thread
From: George Dunlap @ 2017-10-13 9:10 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Julien Grall, Wei Liu, xen-devel, Ian Jackson
On 10/13/2017 10:06 AM, Jan Beulich wrote:
>>>> On 13.10.17 at 11:00, <george.dunlap@citrix.com> wrote:
>> Changeset XXXX introduced "batch mode" to afl-harness, which allowed
>
> With (part of) the commit hash and the title inserted here and ...
Gah. :-)
>
>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> @@ -99,13 +99,17 @@ int main(int argc, char **argv)
>> exit(-1);
>> }
>>
>> - if ( !feof(fp) )
>> + /* Only run the test if the input file was smaller than INPUT_SIZE */
>> + if ( feof(fp) )
>> + {
>> + LLVMFuzzerTestOneInput(input, size);
>> + }
>
> ... ideally with the unnecessary braces dropped here
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Do you really want this to look like this?
if ( ... )
foo();
else
{
...
}
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 9:06 ` Jan Beulich
2017-10-13 9:10 ` George Dunlap
@ 2017-10-13 9:12 ` George Dunlap
1 sibling, 0 replies; 9+ messages in thread
From: George Dunlap @ 2017-10-13 9:12 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Julien Grall, Wei Liu, xen-devel, Ian Jackson
On 10/13/2017 10:06 AM, Jan Beulich wrote:
>>>> On 13.10.17 at 11:00, <george.dunlap@citrix.com> wrote:
>> Changeset XXXX introduced "batch mode" to afl-harness, which allowed
>
> With (part of) the commit hash and the title inserted here and ...
This should be `2b1cde7783` BTW.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 9:10 ` George Dunlap
@ 2017-10-13 9:20 ` Jan Beulich
2017-10-13 10:23 ` George Dunlap
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2017-10-13 9:20 UTC (permalink / raw)
To: George Dunlap
Cc: Andrew Cooper, Julien Grall, Wei Liu, xen-devel, Ian Jackson
>>> On 13.10.17 at 11:10, <george.dunlap@citrix.com> wrote:
> On 10/13/2017 10:06 AM, Jan Beulich wrote:
>>>>> On 13.10.17 at 11:00, <george.dunlap@citrix.com> wrote:
>>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>> @@ -99,13 +99,17 @@ int main(int argc, char **argv)
>>> exit(-1);
>>> }
>>>
>>> - if ( !feof(fp) )
>>> + /* Only run the test if the input file was smaller than INPUT_SIZE */
>>> + if ( feof(fp) )
>>> + {
>>> + LLVMFuzzerTestOneInput(input, size);
>>> + }
>>
>> ... ideally with the unnecessary braces dropped here
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> Do you really want this to look like this?
>
> if ( ... )
> foo();
> else
> {
> ...
> }
Yes. It's Linux and qemu who dislike non-matched if/else bodies,
but our ./CODING_STYLE only says
"Braces should be omitted for blocks with a single statement. e.g.,
if ( condition )
single_statement();"
and personally I'm happy that it doesn't say anything more.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 9:20 ` Jan Beulich
@ 2017-10-13 10:23 ` George Dunlap
2017-10-13 10:31 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: George Dunlap @ 2017-10-13 10:23 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Julien Grall, Wei Liu, xen-devel, Ian Jackson
On 10/13/2017 10:20 AM, Jan Beulich wrote:
>>>> On 13.10.17 at 11:10, <george.dunlap@citrix.com> wrote:
>> On 10/13/2017 10:06 AM, Jan Beulich wrote:
>>>>>> On 13.10.17 at 11:00, <george.dunlap@citrix.com> wrote:
>>>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>>> @@ -99,13 +99,17 @@ int main(int argc, char **argv)
>>>> exit(-1);
>>>> }
>>>>
>>>> - if ( !feof(fp) )
>>>> + /* Only run the test if the input file was smaller than INPUT_SIZE */
>>>> + if ( feof(fp) )
>>>> + {
>>>> + LLVMFuzzerTestOneInput(input, size);
>>>> + }
>>>
>>> ... ideally with the unnecessary braces dropped here
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> Do you really want this to look like this?
>>
>> if ( ... )
>> foo();
>> else
>> {
>> ...
>> }
>
> Yes. It's Linux and qemu who dislike non-matched if/else bodies,
> but our ./CODING_STYLE only says
>
> "Braces should be omitted for blocks with a single statement. e.g.,
>
> if ( condition )
> single_statement();"
>
> and personally I'm happy that it doesn't say anything more.
Hmm, I personally think it's ugly enough that I'd rather restructure the
code to avoid it looking like that. :-)
I'll see what I can do.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 10:23 ` George Dunlap
@ 2017-10-13 10:31 ` Jan Beulich
2017-10-13 10:36 ` George Dunlap
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2017-10-13 10:31 UTC (permalink / raw)
To: George Dunlap
Cc: Andrew Cooper, Julien Grall, Wei Liu, xen-devel, Ian Jackson
>>> On 13.10.17 at 12:23, <george.dunlap@citrix.com> wrote:
> On 10/13/2017 10:20 AM, Jan Beulich wrote:
>>>>> On 13.10.17 at 11:10, <george.dunlap@citrix.com> wrote:
>>> On 10/13/2017 10:06 AM, Jan Beulich wrote:
>>>>>>> On 13.10.17 at 11:00, <george.dunlap@citrix.com> wrote:
>>>>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>>>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>>>> @@ -99,13 +99,17 @@ int main(int argc, char **argv)
>>>>> exit(-1);
>>>>> }
>>>>>
>>>>> - if ( !feof(fp) )
>>>>> + /* Only run the test if the input file was smaller than INPUT_SIZE */
>>>>> + if ( feof(fp) )
>>>>> + {
>>>>> + LLVMFuzzerTestOneInput(input, size);
>>>>> + }
>>>>
>>>> ... ideally with the unnecessary braces dropped here
>>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Do you really want this to look like this?
>>>
>>> if ( ... )
>>> foo();
>>> else
>>> {
>>> ...
>>> }
>>
>> Yes. It's Linux and qemu who dislike non-matched if/else bodies,
>> but our ./CODING_STYLE only says
>>
>> "Braces should be omitted for blocks with a single statement. e.g.,
>>
>> if ( condition )
>> single_statement();"
>>
>> and personally I'm happy that it doesn't say anything more.
>
> Hmm, I personally think it's ugly enough that I'd rather restructure the
> code to avoid it looking like that. :-)
>
> I'll see what I can do.
Well, assuming you would think that way I've intentionally said
"ideally", i.e. if you really don't want to change it, I can live with
the braces.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 10:31 ` Jan Beulich
@ 2017-10-13 10:36 ` George Dunlap
0 siblings, 0 replies; 9+ messages in thread
From: George Dunlap @ 2017-10-13 10:36 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Julien Grall, Wei Liu, xen-devel, Ian Jackson
On 10/13/2017 11:31 AM, Jan Beulich wrote:
>>>> On 13.10.17 at 12:23, <george.dunlap@citrix.com> wrote:
>> On 10/13/2017 10:20 AM, Jan Beulich wrote:
>>>>>> On 13.10.17 at 11:10, <george.dunlap@citrix.com> wrote:
>>>> On 10/13/2017 10:06 AM, Jan Beulich wrote:
>>>>>>>> On 13.10.17 at 11:00, <george.dunlap@citrix.com> wrote:
>>>>>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>>>>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>>>>>> @@ -99,13 +99,17 @@ int main(int argc, char **argv)
>>>>>> exit(-1);
>>>>>> }
>>>>>>
>>>>>> - if ( !feof(fp) )
>>>>>> + /* Only run the test if the input file was smaller than INPUT_SIZE */
>>>>>> + if ( feof(fp) )
>>>>>> + {
>>>>>> + LLVMFuzzerTestOneInput(input, size);
>>>>>> + }
>>>>>
>>>>> ... ideally with the unnecessary braces dropped here
>>>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>>>
>>>> Do you really want this to look like this?
>>>>
>>>> if ( ... )
>>>> foo();
>>>> else
>>>> {
>>>> ...
>>>> }
>>>
>>> Yes. It's Linux and qemu who dislike non-matched if/else bodies,
>>> but our ./CODING_STYLE only says
>>>
>>> "Braces should be omitted for blocks with a single statement. e.g.,
>>>
>>> if ( condition )
>>> single_statement();"
>>>
>>> and personally I'm happy that it doesn't say anything more.
>>
>> Hmm, I personally think it's ugly enough that I'd rather restructure the
>> code to avoid it looking like that. :-)
>>
>> I'll see what I can do.
>
> Well, assuming you would think that way I've intentionally said
> "ideally", i.e. if you really don't want to change it, I can live with
> the braces.
OK, thanks. :-)
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak
2017-10-13 9:00 [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak George Dunlap
2017-10-13 9:06 ` Jan Beulich
@ 2017-10-17 13:39 ` Julien Grall
1 sibling, 0 replies; 9+ messages in thread
From: Julien Grall @ 2017-10-17 13:39 UTC (permalink / raw)
To: George Dunlap, xen-devel
Cc: Andrew Cooper, Julien Grall, Wei Liu, Jan Beulich, Ian Jackson
Hi George,
On 13/10/17 10:00, George Dunlap wrote:
> Changeset XXXX introduced "batch mode" to afl-harness, which allowed
> the handling of several inputs in sequence.
>
> Unfortunately, it introduced a file pointer leak when the file was
> larger than the maximum size. Restructure the code to always close fp
> if we opened it.
>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> Release exception justification:
> - This is a bug fix. The problem is relatively minor, but the fix is relatively minor too.
I agree here:
Release-acked-by: Julien Grall <julien.grall@linaro.org>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-10-17 13:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-13 9:00 [PATCH for-4.10] fuzz/x86_emulate: Fix afl-harness batch mode file pointer leak George Dunlap
2017-10-13 9:06 ` Jan Beulich
2017-10-13 9:10 ` George Dunlap
2017-10-13 9:20 ` Jan Beulich
2017-10-13 10:23 ` George Dunlap
2017-10-13 10:31 ` Jan Beulich
2017-10-13 10:36 ` George Dunlap
2017-10-13 9:12 ` George Dunlap
2017-10-17 13:39 ` Julien Grall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).