* [PATCH] fuzz: Disable QEMU's signal handlers @ 2020-10-13 15:29 Alexander Bulekov 2020-10-13 16:50 ` Darren Kenny 0 siblings, 1 reply; 6+ messages in thread From: Alexander Bulekov @ 2020-10-13 15:29 UTC (permalink / raw) To: qemu-devel Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, Bandan Das, Stefan Hajnoczi, Paolo Bonzini, dimastep 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fuzz: Disable QEMU's signal handlers 2020-10-13 15:29 [PATCH] fuzz: Disable QEMU's signal handlers Alexander Bulekov @ 2020-10-13 16:50 ` Darren Kenny 2020-10-13 16:52 ` Daniel P. Berrangé 2020-10-13 17:02 ` Alexander Bulekov 0 siblings, 2 replies; 6+ messages in thread From: Darren Kenny @ 2020-10-13 16:50 UTC (permalink / raw) To: Alexander Bulekov, qemu-devel Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, Bandan Das, Stefan Hajnoczi, Paolo Bonzini, dimastep Hi Alex, This mentions the use of atexit() to perform some cleanup, but I'm not seeing that being added here, should it be? 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fuzz: Disable QEMU's signal handlers 2020-10-13 16:50 ` Darren Kenny @ 2020-10-13 16:52 ` Daniel P. Berrangé 2020-10-14 9:09 ` Darren Kenny 2020-10-13 17:02 ` Alexander Bulekov 1 sibling, 1 reply; 6+ messages in thread From: Daniel P. Berrangé @ 2020-10-13 16:52 UTC (permalink / raw) To: Darren Kenny Cc: Laurent Vivier, Thomas Huth, qemu-devel, Alexander Bulekov, Bandan Das, Stefan Hajnoczi, Paolo Bonzini, dimastep 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. > > 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 :| ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fuzz: Disable QEMU's signal handlers 2020-10-13 16:52 ` Daniel P. Berrangé @ 2020-10-14 9:09 ` Darren Kenny 2020-10-14 13:40 ` Alexander Bulekov 0 siblings, 1 reply; 6+ messages in thread From: Darren Kenny @ 2020-10-14 9:09 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Laurent Vivier, Thomas Huth, qemu-devel, Alexander Bulekov, Bandan Das, Stefan Hajnoczi, Paolo Bonzini, dimastep 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: - 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: 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 :| ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fuzz: Disable QEMU's signal handlers 2020-10-14 9:09 ` Darren Kenny @ 2020-10-14 13:40 ` Alexander Bulekov 0 siblings, 0 replies; 6+ messages in thread From: Alexander Bulekov @ 2020-10-14 13:40 UTC (permalink / raw) To: Darren Kenny Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé, qemu-devel, Bandan Das, Stefan Hajnoczi, Paolo Bonzini, dimastep 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: > > - 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 :| ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fuzz: Disable QEMU's signal handlers 2020-10-13 16:50 ` Darren Kenny 2020-10-13 16:52 ` Daniel P. Berrangé @ 2020-10-13 17:02 ` Alexander Bulekov 1 sibling, 0 replies; 6+ messages in thread From: Alexander Bulekov @ 2020-10-13 17:02 UTC (permalink / raw) To: Darren Kenny Cc: Laurent Vivier, Thomas Huth, qemu-devel, Bandan Das, Stefan Hajnoczi, Paolo Bonzini, dimastep On 201013 1750, 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? > That sentence was not clear.. I meant that the developer can (optionally) use atexit when writing a new fuzz-target to perform cleanup. For example see: https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg03735.html It would be nice to clean-up the drive files created in that fuzzer with atexit. However, right now that will not do any good, since clean exit signals are ignored. -Alex > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-14 13:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-13 15:29 [PATCH] fuzz: Disable QEMU's signal handlers Alexander Bulekov 2020-10-13 16:50 ` Darren Kenny 2020-10-13 16:52 ` Daniel P. Berrangé 2020-10-14 9:09 ` Darren Kenny 2020-10-14 13:40 ` Alexander Bulekov 2020-10-13 17:02 ` Alexander Bulekov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).