Skip to content

Commit c9c1934

Browse files
committed
Fix GH-8517: FPM child pointer can be potentially uninitialized
There might be a moment when the child log event is executed after freeing a child. That could possibly happen if the child output is triggered at the same as the terminating of the child. Then the output event could be potentially processed after the terminating event which would cause this kind of issue. The issue might got more visible after introducing the log_stream on a child because it is more likely that this cannot be dereferenced after free. However it is very hard to reproduce this issue so there is no test for this. The fix basically prevents passing a child pointer and instead passes the child PID and then looks the child up by the PID when it is being processed. This is obviously slower but it is a safe way to do it and the slow down should not be hopefully visible in a way that it would overload a master process.
1 parent 1767f32 commit c9c1934

File tree

4 files changed

+12
-6
lines changed

4 files changed

+12
-6
lines changed

NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ PHP NEWS
1515
#66694). (Petr Sumbera)
1616
. Fixed bug #68207 (Setting fastcgi.error_header can result in a WARNING).
1717
(Jakub Zelenka)
18+
. Fixed bug GH-8517 (Random crash of FPM master process in
19+
fpm_stdio_child_said). (Jakub Zelenka)
1820

1921
- MBString:
2022
. Fixed bug GH-9535 (The behavior of mb_strcut in mbstring has been changed in

sapi/fpm/fpm/fpm_children.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ static void fpm_child_unlink(struct fpm_child_s *child) /* {{{ */
120120
}
121121
/* }}} */
122122

123-
static struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
123+
struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
124124
{
125125
struct fpm_worker_pool_s *wp;
126126
struct fpm_child_s *child = 0;

sapi/fpm/fpm/fpm_children.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@
1010
#include "fpm_events.h"
1111
#include "zlog.h"
1212

13+
struct fpm_child_s;
14+
1315
int fpm_children_create_initial(struct fpm_worker_pool_s *wp);
1416
int fpm_children_free(struct fpm_child_s *child);
1517
void fpm_children_bury(void);
1618
int fpm_children_init_main(void);
1719
int fpm_children_make(struct fpm_worker_pool_s *wp, int in_event_loop, int nb_to_spawn, int is_debug);
18-
19-
struct fpm_child_s;
20+
struct fpm_child_s *fpm_child_find(pid_t pid);
2021

2122
struct fpm_child_s {
2223
struct fpm_child_s *prev, *next;

sapi/fpm/fpm/fpm_stdio.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg)
181181
if (!arg) {
182182
return;
183183
}
184-
child = (struct fpm_child_s *)arg;
184+
child = fpm_child_find((intptr_t) arg);
185+
if (!child) {
186+
return;
187+
}
185188

186189
is_stdout = (fd == child->fd_stdout);
187190
if (is_stdout) {
@@ -327,10 +330,10 @@ int fpm_stdio_parent_use_pipes(struct fpm_child_s *child) /* {{{ */
327330
child->fd_stdout = fd_stdout[0];
328331
child->fd_stderr = fd_stderr[0];
329332

330-
fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, child);
333+
fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
331334
fpm_event_add(&child->ev_stdout, 0);
332335

333-
fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, child);
336+
fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
334337
fpm_event_add(&child->ev_stderr, 0);
335338
return 0;
336339
}

0 commit comments

Comments
 (0)