[libvirt] [PATCHv2] virsh: avoid uninitialized memory usage

[Eric Blake]

On 04/19/2012 06:43 PM, Wen Congyang wrote:
> 
> 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.

Thanks for searching.  This is a bug in theory: POSIX requires that you
use sa_sigaction if sa_flags includes SA_SIGINFO, and that you use
sa_handler otherwise, and while it permits sa_handler and sa_sigaction
to overlap, it does not require this.  Thankfully, in practice, I know
of no platform where sa_sigaction and sa_handler do not overlap;
furthermore, I don't know of any commonly used ABIs where casting a
1-arg function pointer and calling it with 3 arguments breaks (the extra
2 args are ignored by the callee); likewise, commonly used ABIs state
that if you cast a 3-arg function pointer and call it with only 1
argument, you have predictable behavior so long as you limit yourself to
the first argument (but you _are_ likely to misbehave if you try to
access the other two arguments).

Again, practice says it will work, but theory says you can't rely on it
to work, so we should fix it.  Do you want to prepare the patch, or shall I?

> 
> 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.

Gnulib doesn't guarante SA_SIGINFO, but it _does_ guarantee that for all
platforms where SA_SIGINFO is not defined, then sa_sigaction overlaps
with sa_handler, and that such platforms have a calling convention .
Therefore, on those platforms, we end up installing a sa_handler, and
given my above statements about practice, as long as we don't access the
second or third argument, we won't trigger any odd behaviors.

> 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.

Yep, definite bug for mingw.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120419/47dade2b/attachment-0001.sig>