Re: [libvirt] [PATCHv2] virsh: avoid uninitialized memory usage
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.