Bug #78038 [Opn->Csd]: Socket_select fails when resource array contains references

Edit report at https://bugs.php.net/bug.php?id=78038&edit=1 ID: 78038 Updated by: nikic@php.net Reported by: ogdg at protonmail dot com Summary: Socket_select fails when resource array contains references -Status: Open +Status: Closed Type: Bug Package: Sockets related Operating System: Fedora 30 PHP Version: 7.3.5 Block user comment: N Private report: N New Comment: Automatic comment on behalf of nikita.ppv@gmail.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=9a74b23297167ca7956fd21428c8b987e7d99aa5 Log: Fixed bug #78038 socket_select with references Previous Comments: ------------------------------------------------------------------------ [2019-05-23 07:58:49] nikic@php.net > So in this case, 6a fails with the warnings listed in the original report, while 6b works fine. > In the actual application, the socket array is passed to a custom function by reference...but I did > try passing it directly and the result was the same in 6a. From looking at the code, I suspect that the problem does have something to do with references being introduced somewhere. At least we're missing a DEREF in here: https://github.com/php/php-src/blob/2e4686b5667e9ccbfbfab10452c42f5b818fcfdb/ext/sockets/sockets.c#L906 ------------------------------------------------------------------------ [2019-05-23 07:33:28] ogdg at protonmail dot com @nikic The latest version I was using that did not have this issue was 7.2, and it never happened in any of the version 5 series. I am using the version that is included with Fedora 30 standard repos without any modifications or custom extensions. Just the default install. @daverandom I made a short script to simulate what my application does but the error does not occur. The actual application is using AF_UNIX type sockets for IPC. I do not have the free time to reproduce a full-blown script of this sort right now, but the bug is still present. The application where this issue happens has the basic structure like this: 1) Main process creates AF_UNIX socket 2) Sub process launched, create their own AF_UNIX socket and connect to the main process socket 3) Main process does a socket_accept for new IPC connections 4) Main process uses a this to get PID of child process: $pid = socket_get_option($conn, SOL_SOCKET, 17); $pid = (int)$pid; 5) The main process keeps a copy of the resource in an array indexed by PIDs. $socketArray['ipc'][$pid]; 6a) Problem happens when I take that array and do this: $e = $w = NULL; $r = $socketArray['ipc']; socket_select($r, $w, $e, 0); 6b) To fix this, I had to manually loop through the resource array: foreach($socketArray['ipc'] as $v) $r[] = $v; ... So in this case, 6a fails with the warnings listed in the original report, while 6b works fine. In the actual application, the socket array is passed to a custom function by reference...but I did try passing it directly and the result was the same in 6a. Both print_r and var_dump show that the array is being correctly assigned in 6a, and that $r does indeed contain resources, which is why the fix in 6b works. I can't really tell what the problem is, but it's definitely there and seems to appear in very specific conditions. My first guess was that the $pid wasn't being cast to INT and was being treated as a string, so I cast it to INT and that didn't help. The other thing I thought could be a problem is the large numeric values for the $pid, meaning that for some reason socket_select chokes when the index values of any connection array are above a certain number, but I couldn't reproduce this in a standalone script just using large index values for socket_select...also, the standalone script is not using pcntl_fork as the actual application does. ------------------------------------------------------------------------ [2019-05-22 07:44:17] nikic@php.net One recent change comes to mind here: https://github.com/php/php-src/commit/bdac9ef10d90997f9576564dde693cbef6594142#diff-9df410db21c230cd2ec04a4c95ccd20f However, it is in stream_select(), not socket_select(). When you say "previous versions" do you mean 7.3.4 or some older version? ------------------------------------------------------------------------ [2019-05-20 10:00:10] daverandom@php.net Thank you for this bug report. To properly diagnose the problem, we need a short but complete example script to be able to reproduce this bug ourselves. A proper reproducing script starts with <?php and ends with ?>, is max. 10-20 lines long and does not require any external resources such as databases, etc. If the script requires a database to demonstrate the issue, please make sure it creates all necessary tables, stored procedures etc. Please avoid embedding huge scripts into the report. I cannot reproduce this in 7.3.5 or any other version. Additionally, none of the relevant code in php-src has been changed for several years in 7.3[1] or master[2]. Please recheck your code, var_dump($r) and verify that it contains at least one valid socket resource, and if so please provide a complete reproducing script. There is a lot of code in the wild which utilises a pattern roughly like below - it's likely that if it were broken then a lot of people would be shouting about it :-) $socket = socket_create(/* stuff */); // ...set up the socket $sockets[(int)$socket] = $socket; // Add socket to array indexed by resource ID $w = $e = NULL; $r = $sockets; $check = socket_select($r, $w, $e, 0); [1] https://github.com/php/php-src/blame/PHP-7.3/ext/sockets/sockets.c#L915-L977 [2] https://github.com/php/php-src/blame/master/ext/sockets/sockets.c#L921-L983 ------------------------------------------------------------------------ [2019-05-20 09:27:50] ogdg at protonmail dot com Description: ------------ This bug occurs in PHP CLI 7.3.5! When socket connection resources are indexed in an array by non-sequential numeric keys, socket_select throws a warning and does not actually accept the array even though it contains valid socket resources. By manually re-indexing the array to contain sequentially numbered keys, the problem disappears. This could a problem with large key values, or non sequential keys, or both. The problem did not exist in previous versions of PHP, and it is not valid for socket_select to require arrays that are sequentially numbered, as there are reasons to index connection arrays by process ID or some other meaningful number. Test script: --------------- # Assume each $connN variable is a valid socket resource # Note that array keys are INT and not string type $connections = array(1235 => $conn1, 2084 => $conn2, 3509 => $conn3); $w = $e = NULL; $r = $connections; $check = socket_select($r, $w, $e, 0); # The ABOVE code fails with bogus warnings in PHP 7.3.5 # The BELOW code works $w = $e = NULL; foreach($connections as $v) $r[] = $v; $check = socket_select($r, $w, $e, 0); Expected result: ---------------- Expect socket_select to work as intended and return a value for $check and a modified connection resource array if appropriate. Actual result: -------------- Result is two warnings: socket_select(): supplied argument is not a valid Socket resource socket_select(): no resource arrays were passed to select Previous versions of PHP, including PHP 7 did not have this problem. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=78038&edit=1