Re: [Qemu-devel] audit needed for signal handlers

[Laszlo Ersek]

On 11/11/13 19:03, Max Filippov wrote:
> 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.

That's incorrect (*). The handler runs on a new *stack frame*, but
inside the same thread. You can specify an alternate stack for signal
handlers to run on in advance (see SA_ONSTACK / sigaltstack()), in which
case the new stack frame will be "allocated" there. Otherwise the system
will just use the normal stack. The handler indeed runs like an
"unexpected", "out-of-the-blue" normal function call.

It is actually pretty vital that the handler is run by the specific
thread that the signal has been delivered to.

(*) Example code (not a correct/portable program due to the race on
"errno", but it does disprove the idea that errno is "protected" on Linux):

  #define _XOPEN_SOURCE 600

  #include <unistd.h>
  #include <errno.h>
  #include <signal.h>
  #include <stdio.h>

  static void ringring(int signum)
  {
    errno = -12;
  }

  int main(void)
  {
    sigaction(SIGALRM,
              &(struct sigaction){ .sa_handler = &ringring },
              NULL);
    alarm(1);

    errno = 0;
    /* pause() would clobber errno */
    while (errno == 0)
      ;

    printf("%d\n", errno);
    return 0;
  }

Thanks
Laszlo