* Re: [PATCH] qga: fix crash when a signal arrives during shutdown
2026-10-31 1:07 [PATCH] qga: fix crash when a signal arrives during shutdown Brad Smith
@ 2026-10-31 12:49 ` Roman Bogorodskiy
0 siblings, 0 replies; 2+ messages in thread
From: Roman Bogorodskiy @ 2026-10-31 12:49 UTC (permalink / raw)
To: Brad Smith; +Cc: Michael Roth, Kostiantyn Kostiuk, qemu-devel
Brad Smith wrote:
> qga: fix crash when a signal arrives during shutdown
For some reason your commit message bodies duplicate the title.
> quit_handler() stays installed for SIGINT and SIGTERM for the life
> of the process and dereferences the global ga_state. cleanup_agent()
> frees the agent state and clears ga_state, so a SIGINT or SIGTERM
> arriving after cleanup makes quit_handler() dereference NULL in
> ga_is_frozen() and the agent crashes with SIGSEGV.
>
> This happens in practice when qemu-ga runs under something that
> forwards signals. For example, running it under sudo and stopping it
> with "pkill -f" signals both sudo and qemu-ga, so the agent receives
> SIGTERM twice.
>
> Reproduced on FreeBSD by sending SIGTERM after cleanup_agent() under
> lldb.
>
> Return early from quit_handler() when ga_state is NULL, and clear
> ga_state as soon as the command state cleanup is done, before the
> main loop and the state are freed, so a signal during the rest of
> cleanup does not touch freed memory. ga_state cannot be cleared
> earlier, as command cleanup callbacks such as
> guest_fsfreeze_cleanup() still use it.
>
> Signed-off-by: Brad Smith <brad@comstyle.com>
> ---
> qga/main.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index fd19c7037d..8cc9810572 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -167,6 +167,10 @@ init_dfl_pathnames(void)
>
> static void quit_handler(int sig)
> {
> + if (!ga_state) {
> + return;
> + }
> +
> /* if we're frozen, don't exit unless we're absolutely forced to,
> * because it's basically impossible for graceful exit to complete
> * unless all log/pid files are on unfreezable filesystems. there's
> @@ -1589,13 +1593,13 @@ static void cleanup_agent(GAState *s)
> ga_command_state_free(s->command_state);
> json_message_parser_destroy(&s->parser);
> }
> + ga_state = NULL;
> g_free(s->pstate_filepath);
> g_free(s->state_filepath_isfrozen);
> if (s->main_loop) {
> g_main_loop_unref(s->main_loop);
> }
> g_free(s);
> - ga_state = NULL;
> }
>
> static int run_agent_once(GAState *s)
> --
> 2.56.0
>
I was testing this change by sending SIGTERM in a loop and noticed that
it still crashes with SIGSEGV or SIGARBT from time to time. I was able
to fix that by ignoring signals in the clean up routines:
diff --git a/qga/main.c b/qga/main.c
index 8cc9810572..47686bca80 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -167,6 +167,12 @@ init_dfl_pathnames(void)
static void quit_handler(int sig)
{
+#ifndef _WIN32
+ struct sigaction action = {
+ .sa_handler = SIG_IGN,
+ };
+#endif
+
if (!ga_state) {
return;
}
@@ -203,6 +209,13 @@ static void quit_handler(int sig)
return;
#endif
}
+
+#ifndef _WIN32
+ sigemptyset(&action.sa_mask);
+ sigaction(SIGINT, &action, NULL);
+ sigaction(SIGTERM, &action, NULL);
+#endif
+
g_debug("received signal num %d, quitting", sig);
stop_agent(ga_state, true);
@@ -1587,6 +1600,18 @@ static void cleanup_agent(GAState *s)
if (s->load_avg_pdh_query) {
PdhCloseQuery(s->load_avg_pdh_query);
}
+#else
+ struct sigaction action = {
+ .sa_handler = SIG_IGN,
+ };
+
+ sigemptyset(&action.sa_mask);
+ if (sigaction(SIGINT, &action, NULL) != 0) {
+ g_warning("error configuring signal handler: %s", strerror(errno));
+ }
+ if (sigaction(SIGTERM, &action, NULL) != 0) {
+ g_warning("error configuring signal handler: %s", strerror(errno));
+ }
#endif
if (s->command_state) {
ga_command_state_cleanup_all(s->command_state);
^ permalink raw reply related [flat|nested] 2+ messages in thread