[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]