Re: [libvirt] [PATCHv2] virnetserver: handle sigaction correctly

POSIX says that sa_sigaction is only safe to use if sa_flags includes SA_SIGINFO; conversely, sa_handler is only safe to use when flags excludes that bit. Gnulib doesn't guarantee an implementation of SA_SIGINFO, but does guarantee that if SA_SIGINFO is undefined, we can safely define it to 0 as long as we don't dereference the 2nd or 3rd argument of any handler otherwise registered via sa_sigaction. Based on a report by Wen Congyang. * src/rpc/virnetserver.c (SA_SIGINFO): Stub for mingw. (virNetServerSignalHandler): Avoid bogus dereference. (virNetServerFatalSignal, virNetServerNew): Set flags properly. --- v2: Simplify according to what gnulib gives us for mingw. src/rpc/virnetserver.c | 28 ++++++++++++++++++++-------- 1 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 3965fc2..3f3989e 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1,7 +1,7 @@ /* * virnetserver.c: generic network RPC server * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -40,6 +40,10 @@ # include "virnetservermdns.h" #endif +#ifndef SA_SIGINFO +# define SA_SIGINFO 0 +#endif + #define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ @@ -270,8 +274,9 @@ error: } -static void virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED, - void *context ATTRIBUTE_UNUSED) +static void +virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED, + void *context ATTRIBUTE_UNUSED) { struct sigaction sig_action; int origerrno; @@ -286,6 +291,7 @@ static void virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED #ifdef SIGUSR2 if (sig != SIGUSR2) { #endif + memset(&sig_action, 0, sizeof(sig_action)); sig_action.sa_handler = SIG_DFL; sigaction(sig, &sig_action, NULL); raise(sig); @@ -363,6 +369,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, * debugging purposes or testing */ sig_action.sa_sigaction = virNetServerFatalSignal; + sig_action.sa_flags = SA_SIGINFO; sigaction(SIGFPE, &sig_action, NULL); sigaction(SIGSEGV, &sig_action, NULL); sigaction(SIGILL, &sig_action, NULL); @@ -420,17 +427,24 @@ static sig_atomic_t sigErrors = 0; static int sigLastErrno = 0; static int sigWrite = -1; -static void virNetServerSignalHandler(int sig, siginfo_t * siginfo, - void* context ATTRIBUTE_UNUSED) +static void +virNetServerSignalHandler(int sig, siginfo_t * siginfo ATTRIBUTE_UNUSED,

+ void* context ATTRIBUTE_UNUSED) { int origerrno; int r; + siginfo_t tmp; + + if (SA_SIGINFO) + tmp = *siginfo; + else + memset(&tmp, 0, sizeof(tmp)); /* set the sig num in the struct */ - siginfo->si_signo = sig; + tmp->si_signo = sig;

origerrno = errno; - r = safewrite(sigWrite, siginfo, sizeof(*siginfo)); + r = safewrite(sigWrite, &tmp, sizeof(tmp)); if (r == -1) { sigErrors++; sigLastErrno = errno; @@ -533,9 +547,7 @@ int virNetServerAddSignalHandler(virNetServerPtr srv, memset(&sig_action, 0, sizeof(sig_action)); sig_action.sa_sigaction = virNetServerSignalHandler; -#ifdef SA_SIGINFO sig_action.sa_flags = SA_SIGINFO; -#endif sigemptyset(&sig_action.sa_mask); sigaction(signum, &sig_action, &sigdata->oldaction);