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

[Laszlo Ersek <lersek@redhat.com>]

On 11/11/13 18:47, Paolo Bonzini wrote:
> Il 11/11/2013 18:13, Peter Maydell ha scritto:
>>>> That said, aren't all signals in QEMU (except SIG_IPI) caught with
>>>> signalfd and the handlers run synchronously in the iothread?
>> Eric specifically points out one which is not.
>> (I'm pretty sure that 'reinstall signal handler at
>> end of signal handler' is ancient voodoo that we don't
>> want either, incidentally.)
> 
> Yeah, I was convinced it was---I still cannot find a reason why SIGWINCH
> needs to be handled synchronously.
> 
> resize_term is definitely not signal safe; the man page reflects
> 10-year-old (or more) signal handling lore: "While these functions are
> intended to be used to support a signal handler (i.e., for SIGWINCH),
> care should be taken to avoid invoking them in a context where malloc or
> realloc may have been interrupted, since it uses those functions".
> 
> Calling malloc/realloc from a signal handler is taboo these days...

The problem is when you call a non-async-signal-safe from the handler
and you've interrupted another non-async-signal-safe function. Ie.
there's trouble *only* if both interruptor and interruptee are
non-async-signal-safe.

If at least one is async-signal-safe, then there's no problem. The more
frequent case for this is when the handler is async-signal-safe (like
when it sets a volatile sig_atomic_t flag, and/or uses local variables
exclusively and possibly calls other async-signal-safe functions).

But, the reverse setup exists as well, when you do whatever you like in
the signal handler, just restrict it (ie. the signal delivery) to
portions of the "normal" source where no non-async-signal-safe functions
can run. One example is when you block the signal for the entire event
loop body, and you unblock it only atomically in pselect() or ppoll().
If there has been a signal pending, the (non-async-signal-safe) handler
body will run *inside* pselect(), but that's fine.

(Of course you still need to care about *other* signals interrupting the
handler, see also sa_mask.)

It's by far the best to do what Eric suggests (and I usually combine
that even with the "block it everywhere" idea, because I hate EINTR --
in a correct event loop no function call should block anyway, and EINTR
is just a nuisance).

I probably glossed over a thousand details and I'll get shredded for
that, but that's OK. :)

Done ranting :)

Thanks,
Laszlo