[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] audit needed for signal handlers
From: |
Max Filippov |
Subject: |
Re: [Qemu-devel] audit needed for signal handlers |
Date: |
Mon, 11 Nov 2013 22:03:18 +0400 |
On Mon, Nov 11, 2013 at 8:50 PM, Eric Blake <address@hidden> wrote:
> 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)
An unsafe function is called in a signal. See man 7 signal,
section 'Async-signal-safe functions'. This should be avoided.
> 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.
I believe it cannot, at least in linux, as technically the signal
handler is always called in a new thread, specifically created
to only handle that signal, and errno should be thread-local.
> 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.
--
Thanks.
-- Max
- Re: [Qemu-devel] audit needed for signal handlers, (continued)
- Re: [Qemu-devel] audit needed for signal handlers, Eric Blake, 2013/11/11
- Re: [Qemu-devel] audit needed for signal handlers, Paolo Bonzini, 2013/11/11
- Re: [Qemu-devel] audit needed for signal handlers, Peter Maydell, 2013/11/11
- Re: [Qemu-devel] audit needed for signal handlers, Eric Blake, 2013/11/11
- Re: [Qemu-devel] audit needed for signal handlers, Paolo Bonzini, 2013/11/11
- Re: [Qemu-devel] audit needed for signal handlers, Gerd Hoffmann, 2013/11/12
- Re: [Qemu-devel] audit needed for signal handlers, Laszlo Ersek, 2013/11/12
- Re: [Qemu-devel] audit needed for signal handlers, Eric Blake, 2013/11/11
- Re: [Qemu-devel] audit needed for signal handlers, Paolo Bonzini, 2013/11/11
Re: [Qemu-devel] audit needed for signal handlers, Peter Maydell, 2013/11/11
Re: [Qemu-devel] audit needed for signal handlers,
Max Filippov <=