php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #52784 Race condition when handling many concurrent signals
Submitted: 2010-09-06 18:09 UTC Modified: 2010-11-01 23:44 UTC
Votes:2
Avg. Score:4.5 ± 0.5
Reproduced:2 of 2 (100.0%)
Same Version:2 (100.0%)
Same OS:2 (100.0%)
From: nick dot telford at gmail dot com Assigned: lbarnaud (profile)
Status: Closed Package: PCNTL related
PHP Version: 5.3.3 OS: Linux 2.6+
Private report: No CVE-ID: None
 [2010-09-06 18:09 UTC] nick dot telford at gmail dot com
Description:
------------
When a user-defined signal handler has been defined and many concurrent signals 
are being delivered to it through ext/pcntl, a race-condition can occur causing 
memory corruption.

ext/pcntl handles signals to user-defined functions by placing incoming signals 
in to a "pending signals queue" (linked-list), which is then iterated when 
pcntl_signal_dispatch() is called (either explicitly, or via a tick).

However, if another signal interrupts the execution of the signal handler whilst 
it is manipulating this linked list, it can become corrupted.

The correct solution is to block signals from being delivered to the handler 
whilst it is handling a signal. This is achieved using sigprocmask(SIG_BLOCK).

We have two patches, the first just cleans up the allocation of the pending 
signals queue. The second directly addresses the problem by wrapping any block 
of code that alters the pending signal queue with a set of syscalls to block the 
handling of any new signals.

This is the safest approach as blocking the signals will cause the kernel to 
queue and deliver them once the handler has been unblocked.


Patches

pcntl_pending_signal_allication.diff (last revision 2010-09-06 16:10 UTC by nick dot telford at gmail dot com)
pcntl_sigprocmask.diff (last revision 2010-09-06 16:09 UTC by nick dot telford at gmail dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2010-09-06 18:15 UTC] nick dot telford at gmail dot com
It's been suggested that signal.h isn't portable and should be checked for in 
config.m4.

Thing is, if this functionality is disabled, there exists a potentially crippling 
race condition. Given the small number of people who actually use this extension, 
I would recommend that signal.h/sigprocmask etc. should be a pre-requisite for 
activating custom signal handling. Without it, all userland signal handling 
should be disabled - thoughts?
 [2010-09-09 02:13 UTC] felipe@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: lbarnaud
 [2010-11-01 23:29 UTC] lbarnaud@php.net
Automatic comment from SVN on behalf of lbarnaud
Revision: http://svn.php.net/viewvc/?view=revision&revision=305018
Log: fixed bug #52784 (Race condition when handling many
concurrent signals)
 [2010-11-01 23:44 UTC] lbarnaud@php.net
Automatic comment from SVN on behalf of lbarnaud
Revision: http://svn.php.net/viewvc/?view=revision&revision=305020
Log: MFH fixed bug #52784 (Race condition when handling many
concurrent signals)
 [2010-11-01 23:44 UTC] lbarnaud@php.net
-Status: Assigned +Status: Closed
 [2010-11-01 23:44 UTC] lbarnaud@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

Thanks !
 [2011-02-14 18:06 UTC] lbarnaud@php.net
Automatic comment from SVN on behalf of lbarnaud
Revision: http://svn.php.net/viewvc/?view=revision&revision=308329
Log: MFH fixed bug #52784 (Race condition when handling many
concurrent signals)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 02:01:28 2024 UTC