[Qemu-devel] audit needed for signal handlers

[Eric Blake]

Quick - identify the bug in this code (from ui/curses.c):

static void curses_winch_handler(int signum)
{
    struct winsize {
        unsigned short ws_row;
        unsigned short ws_col;
        unsigned short ws_xpixel;   /* unused */
        unsigned short ws_ypixel;   /* unused */
    } ws;

    /* terminal size changed */
    if (ioctl(1, TIOCGWINSZ, &ws) == -1)
        return;

    resize_term(ws.ws_row, ws.ws_col);
    curses_calc_pad();
    invalidate = 1;

    /* some systems require this */
    signal(SIGWINCH, curses_winch_handler);
}

Here's a hint: ioctl() can clobber errno.  But if a signal handler is
called in the middle of other code that is using errno, then the handler
MUST restore the value of errno before returning, if it is to guarantee
that the interrupted context won't be corrupted.

More reading on the topic:
https://plus.google.com/+LennartPoetteringTheOneAndOnly/posts/gHSscCJkakd

I have not done a full audit of qemu's signal handlers, so much as a
quick look to see if I could find violations; it was surprisingly easy
to find a bad example.  A signal handler that resets the signal to
SIG_DFL then calls raise() is exempt from caring about errno, but any
signal handler that can fall through to the end and return execution to
the caller MUST ensure that errno is left unchanged, for errno to be
useful in the remaining body of code.  Which is why the best signal
handlers tend to be the one that only flag a volatile variable that is
later checked at safe points of execution, rather than trying to make
complex calls from within the handler context.

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

signature.asc
Description: OpenPGP digital signature