Skip to content

Commit

Permalink
Fixed bug #73342
Browse files Browse the repository at this point in the history
Directly listen on socket, instead of duping it to STDIN and
listening on that.
  • Loading branch information
nikic committed Jun 20, 2018
1 parent 5dd1ef9 commit 69dee5c
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 6 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ PHP NEWS
- Date:
. Fixed bug #76462 (Undefined property: DateInterval::$f). (Anatol)

- FPM:
. Fixed bug #73342 (Vulnerability in php-fpm by changing stdin to
non-blocking). (Nikita)

22 Jun 2019, PHP 7.1.19

- CLI Server:
Expand Down
1 change: 1 addition & 0 deletions sapi/fpm/fpm/fpm_children.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ static struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
{
fpm_globals.max_requests = wp->config->pm_max_requests;
fpm_globals.listening_socket = dup(wp->listening_socket);

This comment has been minimized.

Copy link
@setharnold

setharnold Feb 19, 2020

What happens if this dup() call fails? Thanks


if (0 > fpm_stdio_init_child(wp) ||
0 > fpm_log_init_child(wp) ||
Expand Down
6 changes: 0 additions & 6 deletions sapi/fpm/fpm/fpm_stdio.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,6 @@ int fpm_stdio_init_child(struct fpm_worker_pool_s *wp) /* {{{ */
fpm_globals.error_log_fd = -1;
zlog_set_fd(-1);

if (wp->listening_socket != STDIN_FILENO) {
if (0 > dup2(wp->listening_socket, STDIN_FILENO)) {
zlog(ZLOG_SYSERROR, "failed to init child stdio: dup2()");
return -1;
}
}
return 0;
}
/* }}} */
Expand Down
46 changes: 46 additions & 0 deletions sapi/fpm/tests/bug73342-nonblocking-stdio.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
--TEST--
FPM: bug73342 - Non-blocking stdin
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

require_once "tester.inc";

$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
[unconfined]
listen = {{ADDR}}
pm = dynamic
pm.max_children = 5
pm.start_servers = 1
pm.min_spare_servers = 1
pm.max_spare_servers = 3
EOT;

$code = <<<EOT
<?php
echo "Before\n";
stream_set_blocking(fopen('php://stdin', 'r'), false);
echo "After\n";
EOT;

$tester = new FPM\Tester($cfg, $code);
$tester->start();
$tester->expectLogStartNotices();
$tester->request()->expectBody("Before\nAfter");
$tester->request()->expectBody("Before\nAfter");
$tester->terminate();
$tester->expectLogTerminatingNotices();
$tester->close();

?>
Done
--EXPECT--
Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>

3 comments on commit 69dee5c

@kirotawa
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity,
When this patch is applied in php7.0 bug such as out of memory consumption happened.
Is there any other patches to fix this issue that should be applied to 7.0 to not have this issue?

Bug against 7.0 was reported here: https://bugs.launchpad.net/ubuntu/+source/php7.0/+bug/1863881

@nikic
Copy link
Member Author

@nikic nikic commented on 69dee5c Feb 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirotawa Maybe you are looking for cc5c51e.

@kirotawa
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nikic I`ll try this one and re-test check!!!

Please sign in to comment.