All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Make timeouts more robust
@ 2021-08-04 13:56 Alexander Bulekov
  2021-08-04 13:56 ` [PATCH 1/2] fuzz: use ITIMER_REAL for timeouts Alexander Bulekov
  2021-08-04 13:56 ` [PATCH 2/2] fuzz: unblock SIGALRM so the timeout works Alexander Bulekov
  0 siblings, 2 replies; 5+ messages in thread
From: Alexander Bulekov @ 2021-08-04 13:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexander Bulekov, Paolo Bonzini, Philippe Mathieu-Daudé,
	stefanha, Darren Kenny

Based-on: <20210713150037.9297-1-alxndr@bu.edu>

This is an attempt to fix coverage-build failures on OSS-Fuzz. These builds
broke soon after we added the generic-fuzzer, and have been broken since.
We have little visibility into the issue on the OSS-Fuzz infrastructure, but it
appears to be due to some-sort of timeout during corpus merging. To debug this
issue, I downloaded a copy of all of the corpuses on OSS-Fuzz.
Then, I ran a merge job for each fuzzer-config, using the libfuzzer arguments
that I could glean from the clusterfuzz source:

timeout 79200 ./qemu-fuzz-i386-... -rss_limit_mb=2560 -close_fd_mask=3 \
-max_len=5242880 -timeout=5 -detect_leaks=1 -merge=1 \
./merged/... ./qemu-corpus.clusterfuzz-external.appspot.com/libFuzzer/qemu_...

At the end of the day, there were two jobs still running, both stuck in
fdmon_poll_wait -> qemu_poll_ns -> ppoll
These patches adjust the timeout setup to avoid the fuzzer getting stuck in
this code.


Here is an example of such an input from oss-fuzz, for testing:
cat << EOF | base64 -d > input
SEZVWloBAAAAAAAAADc16kZVWlqGRlVaWgZGVVpaz/8PJ4Bg/wAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABNzPqRlVaWghGVVpaBkZVWlrP/w8ngGD/
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/////////////////////////////
/////////////////////////////////////////////++3kP//////////////////////////
/////////////////////////////////////wAAAAAAAAAACQAAAgAAWQELAAC3s7Oz/wjzoIGJ
/////1VaWgZGVVpa33NydP+ROTIyMzM3MjAzNjg1NDc3NTgxMEFzfo0wu3PuAAD9AAABI1onyrgI
RlX/EvIkWloGRlVaWt9zcnT/kTNBc36NzESMEbnZovm5ADdaCFoGRlVaWt/fFwErt7OzswFGkTEt
WiMjI0ZVWlo9/z3/VUZVWloIRtVaWgZGVVpazwAIJ4DvA+/v7+/v/wj/FQAJAAACAABZAQsAAAAA
ALezs7P/CP///0ZVWloGWt/fFwErt7Ozs/8I/wkAAAIAAFkBCyAVAAAAAAAAAFpaWkZVWlrfc3J0
/5EzQXN+jTK7c+5GVVVaWgZGVVpa37Ozs7Ozs7MDAAAAs7Ozs1X4kgP4s7Ozs7OzVfiSA/hoiGIW
/99zcnT/kTNBc36NMrtz7kbPAAAAAAAAACn/s7MDAAAAs7Ozs1X4kgP4s7Ozs7OzVfiSA/hoiGIW
/99zcnT/kTNBc36NMrtz7kbPAAAAAAAAACn/BQhGVVpaBkZVWlrP/w/hf58A//9EjO5GzwAAAAAA
AAAp/wUIRlVaWgZGVVpaz/8PJ4Bg/wAAAAAAAAAAAAAAAZSLi0ZVWlpaWkZVWlrfc3J0ACn/BQhG
VVpaBkZVWlrP/w8ngGD/AAAAAAAAAAAAAAABlIuLRlVaWlpaRlVaWt9zcnT/kTJBc36NMrtz7kZV
VVpaBkZVWlrfs7Ozs7OzswMAAACzs7OzVfiSA/izs7Ozs7NV+JID+GiIYhb/3wAAAAAAt7Ozs/8I
/////1VaWgZa398XASu3s7Oz/wj/CQEAAgAAWQELAAAAAAC3s7Oz/wjQ////VVpaBkZVWlrfc3J0
/5Ez4oGlQXN+jcxEDxG5ovm5ADdaCFoGRlVaWt/fFwErt7Oz7+/v7+/v7+/igZ/v7+/v7x4BAAAA
AAAA7+8gn5+fn5+fn5+fn5+fn5+fnyEAVQCfn5+fn5+fn5+fn58BC0ZVWloIRlVaWgZGAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAClaWgZGVQIAAEZVWlrXvAABdAB0/wZGVVpa17wAAXQAdAgAAEZVWlrX
vAQBdAB0MP82CAAAKf///////////////1paBkZVWlrfc3J0/1paJwB+A0Z+WlpGVVpaWicAfgNG
flpaRlVaWlonAH4DRn5aWkZVWlpaJwB+A0Z+WlpGVVoAAADguYQA//OggLz/////////////Cf//
/1XfRlVaWghGVVpaBkZVWlrPBADLkSf/DAAAAAAABwAAAAAAI1oIRlVaWgZGVVpa398XAf//////
RlVaWv///////wn///9VWlpGVVpa33NydP+RNEFz/////////////////wZGVVpa33Ozs7Ozs7Oz
s7Ozs7Ozs7Ozs64GRlVaWt9zs7Ozs7Ozs7Ozs7Ozs64GRlVaWt9zs7Ozs7Ozs7Ozs7Ozs64GRlVa
Wt9zs7Ozs7Ozs7Ozs7Ozs64GRlVaWt9zs7Ozs7Ozs7Ozs7Ozc3J0/5EzWlrf37Ozs7Ozs+bl////
/////////////1paAQRzs7OzRlVaWghGVVpaBkZVWlqt/////wAAAAEA/QH///9GdDxlVWD//0ZV
Wlow/Q8EAABGVf//RlVaWjf9D7Ozs64GRlVaWt9zs7MEBAAARlX//0ZVWlo3/Q8E//8CBHMARlX/
/0ZVWlo3/Q8EAABGVf//RlVaWjf9DwT//wIEc2Vtc0ZVYP//RlVaWjf9DwQAAEZVRlVaWlpaN/0P
BP//AgRzAEZV//9GVVpaN/0PBAAARlX//0ZVWlo3/Q8E//8CBHNlbXNGVWD//0ZVWlo0/Q9bs/8B
L7Ozs7Ozs7Ozs7OzrgZGVVoaILYg/v//vUZVWloIRmD//0ZVWlo0/Q9bWrP/AS8aILYg/v//vUZV
WloIRlVaWghGVVpaBkZVWlr/tiAa4EQg/v9Bf71GVVpaswizRlVaWghGVVpaBkZVWlqtEQBGVVpa
CEZVWt9zs7Ozs7Ozs7Ozs7Ozs65aBkZVWlojAAAgbiMjBjNGI1VaI0ZV
EOF

Run it with:
./qemu-fuzz-i386 --fuzz-target=generic-fuzz-ahci-hd ./input

For this to timeout and exit, both of the patches in the series are required.

Alexander Bulekov (2):
  fuzz: use ITIMER_REAL for timeouts
  fuzz: unblock SIGALRM so the timeout works

 tests/qtest/fuzz/generic_fuzz.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.30.2



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] fuzz: use ITIMER_REAL for timeouts
  2021-08-04 13:56 [PATCH 0/2] Make timeouts more robust Alexander Bulekov
@ 2021-08-04 13:56 ` Alexander Bulekov
  2021-08-04 15:34   ` Darren Kenny
  2021-08-04 13:56 ` [PATCH 2/2] fuzz: unblock SIGALRM so the timeout works Alexander Bulekov
  1 sibling, 1 reply; 5+ messages in thread
From: Alexander Bulekov @ 2021-08-04 13:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Darren Kenny,
	Philippe Mathieu-Daudé,
	Alexander Bulekov, Bandan Das, stefanha, Paolo Bonzini

Using ITIMER_VIRTUAL is a bad idea, if the fuzzer hits a blocking
syscall - e.g. ppoll with a NULL timespec. This causes timeout issues
while fuzzing some block-device code. Fix that by using wall-clock time.
This might cause inputs to timeout sometimes due to scheduling
effects/ambient load, but it is better than bringing the entire fuzzing
process to a halt.

Based-on: <20210713150037.9297-1-alxndr@bu.edu>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/generic_fuzz.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 3e8ce29227..de427a3727 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -695,7 +695,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
         while (cmd && Size) {
             /* Reset the timeout, each time we run a new command */
             if (timeout) {
-                setitimer(ITIMER_VIRTUAL, &timer, NULL);
+                setitimer(ITIMER_REAL, &timer, NULL);
             }
 
             /* Get the length until the next command or end of input */
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] fuzz: unblock SIGALRM so the timeout works
  2021-08-04 13:56 [PATCH 0/2] Make timeouts more robust Alexander Bulekov
  2021-08-04 13:56 ` [PATCH 1/2] fuzz: use ITIMER_REAL for timeouts Alexander Bulekov
@ 2021-08-04 13:56 ` Alexander Bulekov
  2021-08-04 15:33   ` Darren Kenny
  1 sibling, 1 reply; 5+ messages in thread
From: Alexander Bulekov @ 2021-08-04 13:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Darren Kenny,
	Philippe Mathieu-Daudé,
	Alexander Bulekov, Bandan Das, stefanha, Paolo Bonzini

The timeout mechanism wont work if SIGALRM is blocked. This changes
unmasks SIGALRM when the timer is installed. This doesn't completely
solve the problem, as the fuzzer could trigger some device activity that
re-masks SIGALRM. However, there are currently no inputs on OSS-Fuzz
that re-mask SIGALRM and timeout. If that turns out to be a real issue,
we could try to hook sigmask-type calls, or use a separate timer thread.

Based-on: <20210713150037.9297-1-alxndr@bu.edu>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/generic_fuzz.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index de427a3727..dd7e25851c 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -670,6 +670,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
     if (fork() == 0) {
         struct sigaction sact;
         struct itimerval timer;
+        sigset_t set;
         /*
          * Sometimes the fuzzer will find inputs that take quite a long time to
          * process. Often times, these inputs do not result in new coverage.
@@ -684,6 +685,10 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
             sact.sa_handler = handle_timeout;
             sigaction(SIGALRM, &sact, NULL);
 
+            sigemptyset(&set);
+            sigaddset(&set, SIGALRM);
+            pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+
             memset(&timer, 0, sizeof(timer));
             timer.it_value.tv_sec = timeout / USEC_IN_SEC;
             timer.it_value.tv_usec = timeout % USEC_IN_SEC;
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] fuzz: unblock SIGALRM so the timeout works
  2021-08-04 13:56 ` [PATCH 2/2] fuzz: unblock SIGALRM so the timeout works Alexander Bulekov
@ 2021-08-04 15:33   ` Darren Kenny
  0 siblings, 0 replies; 5+ messages in thread
From: Darren Kenny @ 2021-08-04 15:33 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Philippe Mathieu-Daudé,
	Alexander Bulekov, Bandan Das, stefanha, Paolo Bonzini

On Wednesday, 2021-08-04 at 09:56:21 -04, Alexander Bulekov wrote:
> The timeout mechanism wont work if SIGALRM is blocked. This changes

NIT: s/wont/won't/
     s/changes/change/

> unmasks SIGALRM when the timer is installed. This doesn't completely
> solve the problem, as the fuzzer could trigger some device activity that
> re-masks SIGALRM. However, there are currently no inputs on OSS-Fuzz
> that re-mask SIGALRM and timeout. If that turns out to be a real issue,
> we could try to hook sigmask-type calls, or use a separate timer thread.
>
> Based-on: <20210713150037.9297-1-alxndr@bu.edu>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

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

> ---
>  tests/qtest/fuzz/generic_fuzz.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index de427a3727..dd7e25851c 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -670,6 +670,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>      if (fork() == 0) {
>          struct sigaction sact;
>          struct itimerval timer;
> +        sigset_t set;
>          /*
>           * Sometimes the fuzzer will find inputs that take quite a long time to
>           * process. Often times, these inputs do not result in new coverage.
> @@ -684,6 +685,10 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>              sact.sa_handler = handle_timeout;
>              sigaction(SIGALRM, &sact, NULL);
>  
> +            sigemptyset(&set);
> +            sigaddset(&set, SIGALRM);
> +            pthread_sigmask(SIG_UNBLOCK, &set, NULL);
> +
>              memset(&timer, 0, sizeof(timer));
>              timer.it_value.tv_sec = timeout / USEC_IN_SEC;
>              timer.it_value.tv_usec = timeout % USEC_IN_SEC;
> -- 
> 2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] fuzz: use ITIMER_REAL for timeouts
  2021-08-04 13:56 ` [PATCH 1/2] fuzz: use ITIMER_REAL for timeouts Alexander Bulekov
@ 2021-08-04 15:34   ` Darren Kenny
  0 siblings, 0 replies; 5+ messages in thread
From: Darren Kenny @ 2021-08-04 15:34 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Philippe Mathieu-Daudé,
	Alexander Bulekov, Bandan Das, stefanha, Paolo Bonzini

On Wednesday, 2021-08-04 at 09:56:20 -04, Alexander Bulekov wrote:
> Using ITIMER_VIRTUAL is a bad idea, if the fuzzer hits a blocking
> syscall - e.g. ppoll with a NULL timespec. This causes timeout issues
> while fuzzing some block-device code. Fix that by using wall-clock time.
> This might cause inputs to timeout sometimes due to scheduling
> effects/ambient load, but it is better than bringing the entire fuzzing
> process to a halt.
>
> Based-on: <20210713150037.9297-1-alxndr@bu.edu>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

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

> ---
>  tests/qtest/fuzz/generic_fuzz.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index 3e8ce29227..de427a3727 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -695,7 +695,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>          while (cmd && Size) {
>              /* Reset the timeout, each time we run a new command */
>              if (timeout) {
> -                setitimer(ITIMER_VIRTUAL, &timer, NULL);
> +                setitimer(ITIMER_REAL, &timer, NULL);
>              }
>  
>              /* Get the length until the next command or end of input */
> -- 
> 2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-04 17:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-04 13:56 [PATCH 0/2] Make timeouts more robust Alexander Bulekov
2021-08-04 13:56 ` [PATCH 1/2] fuzz: use ITIMER_REAL for timeouts Alexander Bulekov
2021-08-04 15:34   ` Darren Kenny
2021-08-04 13:56 ` [PATCH 2/2] fuzz: unblock SIGALRM so the timeout works Alexander Bulekov
2021-08-04 15:33   ` Darren Kenny

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.