Skip to content

Commit 1029537

Browse files
committed
Fix GH-10461: Postpone FPM child freeing in event loop
This is to prevent after free accessing of the child event that might happen when child is killed and the message is delivered at that same time. Also fixes GH-10889 and properly fixes GH-8517 that was not previously fixed correctly.
1 parent 7b76848 commit 1029537

File tree

5 files changed

+36
-10
lines changed

5 files changed

+36
-10
lines changed

NEWS

+4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ PHP NEWS
1414
. Fixed bug GH-10834 (exif_read_data() cannot read smaller stream wrapper
1515
chunk sizes). (nielsdos)
1616

17+
- FPM:
18+
. Fixed bug GH-10461 (PHP-FPM segfault due to after free usage of
19+
child->ev_std(out|err)). (Jakub Zelenka)
20+
1721
- Hash:
1822
. Fixed bug GH-11180 (hash_file() appears to be restricted to 3 arguments).
1923
(nielsdos)

sapi/fpm/fpm/fpm_children.c

+24-1
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,27 @@ static void fpm_child_free(struct fpm_child_s *child) /* {{{ */
6363
}
6464
/* }}} */
6565

66+
static void fpm_postponed_child_free(struct fpm_event_s *ev, short which, void *arg)
67+
{
68+
struct fpm_child_s *child = (struct fpm_child_s *) arg;
69+
70+
if (child->fd_stdout != -1) {
71+
fpm_event_del(&child->ev_stdout);
72+
close(child->fd_stdout);
73+
}
74+
if (child->fd_stderr != -1) {
75+
fpm_event_del(&child->ev_stderr);
76+
close(child->fd_stderr);
77+
}
78+
79+
fpm_child_free((struct fpm_child_s *) child);
80+
}
81+
6682
static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{ */
6783
{
6884
if (child->fd_stdout != -1) {
6985
if (in_event_loop) {
86+
child->postponed_free = true;
7087
fpm_event_fire(&child->ev_stdout);
7188
}
7289
if (child->fd_stdout != -1) {
@@ -76,14 +93,20 @@ static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{
7693

7794
if (child->fd_stderr != -1) {
7895
if (in_event_loop) {
96+
child->postponed_free = true;
7997
fpm_event_fire(&child->ev_stderr);
8098
}
8199
if (child->fd_stderr != -1) {
82100
close(child->fd_stderr);
83101
}
84102
}
85103

86-
fpm_child_free(child);
104+
if (in_event_loop && child->postponed_free) {
105+
fpm_event_set_timer(&child->ev_free, 0, &fpm_postponed_child_free, child);
106+
fpm_event_add(&child->ev_free, 1000);
107+
} else {
108+
fpm_child_free(child);
109+
}
87110
}
88111
/* }}} */
89112

sapi/fpm/fpm/fpm_children.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,13 @@ struct fpm_child_s {
2323
struct fpm_child_s *prev, *next;
2424
struct timeval started;
2525
struct fpm_worker_pool_s *wp;
26-
struct fpm_event_s ev_stdout, ev_stderr;
26+
struct fpm_event_s ev_stdout, ev_stderr, ev_free;
2727
int shm_slot_i;
2828
int fd_stdout, fd_stderr;
2929
void (*tracer)(struct fpm_child_s *);
3030
struct timeval slow_logged;
31-
int idle_kill;
31+
bool idle_kill;
32+
bool postponed_free;
3233
pid_t pid;
3334
int scoreboard_i;
3435
struct zlog_stream *log_stream;

sapi/fpm/fpm/fpm_process_ctl.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ static void fpm_pctl_kill_idle_child(struct fpm_child_s *child) /* {{{ */
318318
if (child->idle_kill) {
319319
fpm_pctl_kill(child->pid, FPM_PCTL_KILL);
320320
} else {
321-
child->idle_kill = 1;
321+
child->idle_kill = true;
322322
fpm_pctl_kill(child->pid, FPM_PCTL_QUIT);
323323
}
324324
}

sapi/fpm/fpm/fpm_stdio.c

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

189186
is_stdout = (fd == child->fd_stdout);
190187
if (is_stdout) {
@@ -277,6 +274,7 @@ static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg)
277274

278275
fpm_event_del(event);
279276

277+
child->postponed_free = true;
280278
if (is_stdout) {
281279
close(child->fd_stdout);
282280
child->fd_stdout = -1;
@@ -330,10 +328,10 @@ int fpm_stdio_parent_use_pipes(struct fpm_child_s *child) /* {{{ */
330328
child->fd_stdout = fd_stdout[0];
331329
child->fd_stderr = fd_stderr[0];
332330

333-
fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
331+
fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, child);
334332
fpm_event_add(&child->ev_stdout, 0);
335333

336-
fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
334+
fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, child);
337335
fpm_event_add(&child->ev_stderr, 0);
338336
return 0;
339337
}

0 commit comments

Comments
 (0)