Re: [PATCH v2] fuzz: Disable QEMU's SIG{INT,HUP,TERM} handlers

From: Paolo Bonzini <pbonzini@redhat.com>
To: Alexander Bulekov <alxndr@bu.edu>, qemu-devel@nongnu.org
Cc: Laurent Vivier <lvivier@redhat.com>,
	Thomas Huth <thuth@redhat.com>,
	Darren Kenny <darren.kenny@oracle.com>,
	Bandan Das <bsd@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	dimastep@yandex-team.ru
Subject: Re: [PATCH v2] fuzz: Disable QEMU's SIG{INT,HUP,TERM} handlers
Date: Thu, 15 Oct 2020 19:10:23 +0200	[thread overview]
Message-ID: <29b563d9-f612-ac8b-ea83-f7eb7b459106@redhat.com> (raw)
In-Reply-To: <20201014142157.46028-1-alxndr@bu.edu>

On 14/10/20 16:21, Alexander Bulekov wrote:
> Prior to this patch, the only way I found to terminate the fuzzer was
> either to:
>  1. Explicitly specify the number of fuzzer runs with the -runs= flag
>  2. SIGKILL the process with "pkill -9 qemu-fuzz-*" or similar
> 
> In addition to being annoying to deal with, SIGKILLing the process skips
> over any exit handlers(e.g. registered with atexit()). This is bad,
> since some fuzzers might create temporary files that should ideally be
> removed on exit using an exit handler. The only way to achieve a clean
> exit now is to specify -runs=N , but the desired "N" is tricky to
> identify prior to fuzzing.
> 
> Why doesn't the process exit with standard SIGINT,SIGHUP,SIGTERM
> signals? QEMU installs its own handlers for these signals in
> os-posix.c:os_setup_signal_handling, which notify the main loop that an
> exit was requested. The fuzzer, however, does not run qemu_main_loop,
> which performs the main_loop_should_exit() check.  This means that the
> fuzzer effectively ignores these signals. As we don't really care about
> cleanly stopping the disposable fuzzer "VM", this patch uninstalls
> QEMU's signal handlers. Thus, we can stop the fuzzer with
> SIG{INT,HUP,TERM} and the fuzzing code can optionally use atexit() to
> clean up temporary files/resources.
> 
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  tests/qtest/fuzz/fuzz.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index d926c490c5..eb0070437f 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -217,5 +217,13 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
>      /* re-enable the rcu atfork, which was previously disabled in qemu_init */
>      rcu_enable_atfork();
>  
> +    /*
> +     * Disable QEMU's signal handlers, since we manually control the main_loop,
> +     * and don't check for main_loop_should_exit
> +     */
> +    signal(SIGINT, SIG_DFL);
> +    signal(SIGHUP, SIG_DFL);
> +    signal(SIGTERM, SIG_DFL);
> +
>      return 0;
>  }
> 

Queued, thanks.

Paolo