Re: [PATCH] fuzz: Disable QEMU's signal handlers

[Alexander Bulekov]

On 201014 1009, Darren Kenny wrote:
> On Tuesday, 2020-10-13 at 17:52:46 +01, Daniel P. Berrangé wrote:
> > On Tue, Oct 13, 2020 at 05:50:37PM +0100, Darren Kenny wrote:
> >> Hi Alex,
> >> 
> >> This mentions the use of atexit() to perform some cleanup, but I'm not
> >> seeing that being added here, should it be?
> >
> > The reference to atexit is strange, because it says the only way to
> > kill the fuzzer is SIGKILL, and that won't let atexit handlers run
> > anyway.
> >
> 
> OK, I understand the context now, it is in reference to Dima's patchset:
> 
> - 
> cover.1602078083.git.dimastep@yandex-team.ru/">https://lore.kernel.org/qemu-devel/cover.1602078083.git.dimastep@yandex-team.ru/
> 
> Where Alex suggested using atexit() to clean up the left over files from
> the test.
> 
> And with regard to SIGKILL, I believe it is that today, before Alex's
> patch that is the only way to stop the fuzzer running - which I can
> attest to since I've found it hard to stop in the past :)
> 
> Resetting these signal handlers to the default behaviour would allow the
> process to be terminated and an atexit() used, as Alex mentioned.
> 
> Alex, if you could clarify the commit message, then I feel this does
> make sent to change in the fuzz testing code, so:
> 

Yes - sorry about the confusion.
-Alex

> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> 
> Thanks,
> 
> Darren.
> 
> >> 
> >> Thanks,
> >> 
> >> Darren.
> >> 
> >> On Tuesday, 2020-10-13 at 11:29:20 -04, Alexander Bulekov wrote:
> >> > With the fuzzer, we never call main_loop_should_exit, since we manually
> >> > call main_loop_wait. This means that the only way to terminate the
> >> > fuzzer is with SIGKILL. Disable the signal handlers, so there are
> >> > reasonable ways to terminate the fuzzer and use atexit() to clean-up
> >> > after the fuzzer.
> >> >
> >> > 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;
> >> >  }
> >> > -- 
> >> > 2.28.0
> >> 
> >
> > Regards,
> > Daniel
> > -- 
> > |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange 
> > :|
> > |: https://libvirt.org         -o-            https://fstop138.berrange.com 
> > :|
> > |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange 
> > :|