[ovs-dev] [PATCH] process: block signals while spawning child processes

[Ansis Atteka]

On Tue, May 27, 2014 at 12:52 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Tue, May 27, 2014 at 12:32:50PM -0700, Ansis Atteka wrote:
>> Between fork() and execvp() calls in the process_start()
>> function both child and parent processes share the same
>> file descriptors.  This means that, if a child process
>> received a signal during this time interval, then it could
>> potentially write data to a shared file descriptor.
>>
>> One such example is fatal signal handler, where, if
>> child process received SIGTERM signal, then it would
>> write data into pipe.  Then a read event would occur
>> on the other end of the pipe where parent process is
>> listening and this would make parent process to incorrectly
>> believe that it was the one who received SIGTERM.
>> Also, since parent process never reads data from this
>> pipe, then this bug would make parent process to consume
>> 100% CPU by immediately waking up from the event loop.
>>
>> This patch will help to avoid this problem by blocking
>> signals until child closes all its file descriptors.
>>
>> Signed-off-by: Ansis Atteka <aatteka at nicira.com>
>> Reported-by: Suganya Ramachandran <suganyar at vmware.com>
>> Issue: 1255110
>
> I agree that this fixes a problem.  Thank you for tracking it down and
> coming up with a good fix.
>
> I am nervous about blocking all signals indiscriminately.  Sometimes
> libraries use signals (e.g. pthreads implementations sometimes do) and
> I don't know whether it's a good idea to block them.  PSPP used to
> have functions fatal_signal_block() and fatal_signal_unblock() to
> block the fatal signal handlers, before commit d8b30702057c
> (fatal-signal: Run signal hooks outside of actual signal handlers.).
> We could reintroduce these, or just add a function to inquire of the
> signals that fatal-signal handles (in the form of a signal mask).

I sent PATCHv2 that addresses your concern. The new patch blocks only
fatal signals.

Just curious. Do you think it would make sense to use ppoll() or
signal_fd() instead of "self pipe trick" to handle signals? Not sure
about portability, but it seems that ppoll() at least is POSIX
compliant and was introduced in Linux 2.6.16.