At 04/20/2012 04:29 AM, Eric Blake Wrote:
On 04/19/2012 02:56 AM, Wen Congyang wrote:
At 04/19/2012 04:51 PM, Alex Jia Wrote:
Detected by valgrind.
* tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
+++ b/tools/virsh.c @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
intCaught = 0; sig_action.sa_sigaction = vshCatchInt; + sig_action.sa_flags = 0; sigemptyset(&sig_action.sa_mask); sigaction(SIGINT, &sig_action, &old_sig_action);
ACK
NACK. vshCatchInt is a 3-arg function, and therefore sig_action.sa_flags must include at least SA_SIGINFO. I inadvertently missed a line when copying code from vshWatchJob(); I'll push the obvious v3 patch shortly.
Sorry, I forgot this. I search sig_action(), and found the same problem in the function virNetServerNew(): ==================== memset(&sig_action, 0, sizeof(sig_action)); sig_action.sa_handler = SIG_IGN; sigaction(SIGPIPE, &sig_action, NULL); /* * catch fatal errors to dump a log, also hook to USR2 for dynamic * debugging purposes or testing */ sig_action.sa_sigaction = virNetServerFatalSignal; sigaction(SIGFPE, &sig_action, NULL); sigaction(SIGSEGV, &sig_action, NULL); ==================== We set sig_action.sa_flags to 0 here. And in virsh.c, I found: ==================== /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO # define SA_SIGINFO 0 #endif ==================== Is it safe to use sig_action.sa_sigaction when SA_SIGINFO is not set? I think it is OK if we access siginfo_t if SA_SIGINFO is not 0. But in the function virNetServerAddSignalHandler(): ==================== 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); ==================== virNetServerSignalHandler() will access siginfo_t without any check.