Re: [Xen-devel] [PATCH 2/2] tools/fuzz: Add a cpu-policy fuzzing harness

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Sergey Dyasli <sergey.dyasli@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>, WeiLiu <wl@xen.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 2/2] tools/fuzz: Add a cpu-policy fuzzing harness
Date: Wed, 5 Jun 2019 12:31:34 +0100	[thread overview]
Message-ID: <82bd675c-c7f8-5ead-f328-bca96bc3976f@citrix.com> (raw)
In-Reply-To: <5CF790890200007800235696@prv1-mh.provo.novell.com>

On 05/06/2019 10:51, Jan Beulich wrote:
>>>> On 04.06.19 at 21:51, <andrew.cooper3@citrix.com> wrote:
>> There is now enough complexity that a fuzzing harness is a good idea, and
>> enough supporting logic to implement one which AFL seems happy with.
>>
>> Take the existing recalculate_synth() helper and export it as
>> x86_cpuid_policy_recalc_synth(), as it is needed by the fuzzing harness.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Wei Liu <wl@xen.org>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Sergey Dyasli <sergey.dyasli@citrix.com>
>> ---
>>  tools/fuzz/cpu-policy/.gitignore          |   1 +
>>  tools/fuzz/cpu-policy/Makefile            |  28 +++++
>>  tools/fuzz/cpu-policy/afl-policy-fuzzer.c | 187 ++++++++++++++++++++++++++++++
> Does this want to be accompanied by a ./MAINTAINERS update
> to the X86 section?

Oops yes, and similarly for tools/tests/cpu-policy/

Do you mind if I fold that change in as well?

>
>>  xen/include/xen/lib/x86/cpuid.h           |   5 +
>>  xen/lib/x86/cpuid.c                       |   7 +-
>>  5 files changed, 224 insertions(+), 4 deletions(-)
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one further remark:
>
>> +int main(int argc, char **argv)
>> +{
>> +    FILE *fp = NULL;
>> +
>> +    setbuf(stdin, NULL);
>> +    setbuf(stdout, NULL);
>> +
>> +    while ( true )
>> +    {
>> +        static const struct option opts[] = {
>> +            { "debug", no_argument, NULL, 'd' },
>> +            { "help", no_argument, NULL, 'h' },
>> +            {},
>> +        };
>> +        int c = getopt_long(argc, argv, "hd", opts, NULL);
>> +
>> +        if ( c == -1 )
>> +            break;
>> +
>> +        switch ( c )
>> +        {
>> +        case 'd':
>> +            printf("Enabling debug\n");
>> +            debug = true;
>> +            break;
>> +
>> +        default:
>> +            printf("Bad option %d (%c)\n", c, c);
>> +            exit(-1);
>> +            break;
> Wouldn't 'h' (wrongly) come into here? (The break statement also looks
> to be unnecessary after exit().)

Hmm - something got lost in a rebase.  That was supposed to be default
printing bad option, then falling through into 'h' with usage.

~Andrew

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