Re: [PATCH] fuzz/x86_emulate: fix bounds for input size

From: Paul Semel <semelpaul@gmail.com>
To: George Dunlap <george.dunlap@citrix.com>, xen-devel@lists.xenproject.org
Cc: andrew.cooper3@citrix.com, wei.liu2@citrix.com, JBeulich@suse.com
Subject: Re: [PATCH] fuzz/x86_emulate: fix bounds for input size
Date: Fri, 23 Feb 2018 13:07:10 +0100	[thread overview]
Message-ID: <499e741c-f98d-6658-c479-5a9748d98c1a@gmail.com> (raw)
In-Reply-To: <ff5d4891-1e00-6e8b-aa7b-b874e60e422d@citrix.com>

On 02/23/2018 11:44 AM, George Dunlap wrote:
> Paul, thanks for reporting this!  A couple of comments...
> 
> On 02/22/2018 11:57 PM, Paul Semel wrote:
>> The minimum size for the input size was set to DATA_OFFSET + 1 which was meaning
>> that we were requesting at least one character of the data array to be filled.
>> This is not needed for the fuzzer to get working correctly.
> 
> Sure, but then the emulator will fail the first insn_fetch() callback.
> I understand there's a conceptual purity to testing such an input, but
> is it actually of practical value in finding bugs?
> 

Actually, if you really want to find bugs, you *really* want AFL to have 
control over the whole data field.

>> The maximum size for the input size was set to INPUT_SIZE, which is actually
>> the size of the data array inside the fuzz_corpus structure and so was not
>> abling user (or AFL) to fill in the whole structure. Changing to
>> sizeof(struct fuzz_corpus) correct this problem.
> 
> Good catch; actually INPUT_SIZE is a misnomer; this should really be
> DATA_SIZE.
> 
> I think it's arguable whether the better thing to do there would be to
> take your approach, of extending the accepted input size to fit the
> initial state + data size, or the other approach, of restricting the
> size of the data[] array such that the total structure size doesn't
> exceed our desired INPUT_SIZE (currently 4k).  The first bit of advice
> in `perf_tips.txt` for AFL is "Keep your test cases small", so I'd be
> inclined to go with the second.
> 

I would also go for the second, as I'm pretty sure we're not using the 
whole buffer in one run.

> But in any case, I'm in the process of reworking how the input works;
> you can see the last patch posted here:
> 
> https://marc.info/?l=xen-devel&m=150774448513434
> 

I took a look at your patch, it look really good. The only thing is that 
I think it might be more relevent to also have an option to get the 
maximum input, as I think it's the better way to find bugs as mentionned 
earlier ! 🙂

> I haven't taken a close look but I think it as a side effect it will fix
> this issue.
> 
>   -George
> 

-- 
Paul Semel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel