Re: [PATCH v3 1/4] fuzz: adjust timeout to allow for longer inputs

[Darren Kenny <darren.kenny@oracle.com>]

On Wednesday, 2021-06-23 at 23:45:00 -04, Alexander Bulekov wrote:
> Using a custom timeout is useful to continue fuzzing complex devices,
> even after we run into some slow code-path. However, simply adding a
> fixed timeout to each input effectively caps the maximum input
> length/number of operations at some artificial value. There are two
> major problems with this:
> 1. Some code might only be reachable through long IO sequences.
> 2. Longer inputs can actually be _better_ for performance. While the
>    raw number of fuzzer executions decreases with larger inputs, the
>    number of MMIO/PIO/DMA operation/second actually increases, since
>    were are speding proportionately less time fork()ing.
>
> With this change, we keep the custom-timeout, but we renew it, prior to
> each MMIO/PIO/DMA operation. Thus, we time-out only when a particaly

TYPO: s/particaly/specific/ or s/particaly/particular/ ?

> operation takes a long time.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Otherwise,

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  tests/qtest/fuzz/generic_fuzz.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index cea7d4058e..71d36e8f6f 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -661,15 +661,16 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>      uint8_t op;
>  
>      if (fork() == 0) {
> +        struct sigaction sact;
> +        struct itimerval timer;
>          /*
>           * Sometimes the fuzzer will find inputs that take quite a long time to
>           * process. Often times, these inputs do not result in new coverage.
>           * Even if these inputs might be interesting, they can slow down the
> -         * fuzzer, overall. Set a timeout to avoid hurting performance, too much
> +         * fuzzer, overall. Set a timeout for each command to avoid hurting
> +         * performance, too much
>           */
>          if (timeout) {
> -            struct sigaction sact;
> -            struct itimerval timer;
>  
>              sigemptyset(&sact.sa_mask);
>              sact.sa_flags   = SA_NODEFER;
> @@ -679,13 +680,17 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>              memset(&timer, 0, sizeof(timer));
>              timer.it_value.tv_sec = timeout / USEC_IN_SEC;
>              timer.it_value.tv_usec = timeout % USEC_IN_SEC;
> -            setitimer(ITIMER_VIRTUAL, &timer, NULL);
>          }
>  
>          op_clear_dma_patterns(s, NULL, 0);
>          pci_disabled = false;
>  
>          while (cmd && Size) {
> +            /* Reset the timeout, each time we run a new command */
> +            if (timeout) {
> +                setitimer(ITIMER_VIRTUAL, &timer, NULL);
> +            }
> +
>              /* Get the length until the next command or end of input */
>              nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR));
>              cmd_len = nextcmd ? nextcmd - cmd : Size;
> -- 
> 2.28.0