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.
This commit is contained in:
Jakub Zelenka 2023-04-15 16:07:09 +01:00
parent 7b768485f3
commit 102953735c
No known key found for this signature in database
GPG key ID: 1C0779DC5C0A9DE4
5 changed files with 36 additions and 10 deletions

4
NEWS
View file

@ -14,6 +14,10 @@ PHP NEWS
. Fixed bug GH-10834 (exif_read_data() cannot read smaller stream wrapper
chunk sizes). (nielsdos)
- FPM:
. Fixed bug GH-10461 (PHP-FPM segfault due to after free usage of
child->ev_std(out|err)). (Jakub Zelenka)
- Hash:
. Fixed bug GH-11180 (hash_file() appears to be restricted to 3 arguments).
(nielsdos)

View file

@ -63,10 +63,27 @@ static void fpm_child_free(struct fpm_child_s *child) /* {{{ */
}
/* }}} */
static void fpm_postponed_child_free(struct fpm_event_s *ev, short which, void *arg)
{
struct fpm_child_s *child = (struct fpm_child_s *) arg;
if (child->fd_stdout != -1) {
fpm_event_del(&child->ev_stdout);
close(child->fd_stdout);
}
if (child->fd_stderr != -1) {
fpm_event_del(&child->ev_stderr);
close(child->fd_stderr);
}
fpm_child_free((struct fpm_child_s *) child);
}
static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{ */
{
if (child->fd_stdout != -1) {
if (in_event_loop) {
child->postponed_free = true;
fpm_event_fire(&child->ev_stdout);
}
if (child->fd_stdout != -1) {
@ -76,6 +93,7 @@ static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{
if (child->fd_stderr != -1) {
if (in_event_loop) {
child->postponed_free = true;
fpm_event_fire(&child->ev_stderr);
}
if (child->fd_stderr != -1) {
@ -83,7 +101,12 @@ static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{
}
}
fpm_child_free(child);
if (in_event_loop && child->postponed_free) {
fpm_event_set_timer(&child->ev_free, 0, &fpm_postponed_child_free, child);
fpm_event_add(&child->ev_free, 1000);
} else {
fpm_child_free(child);
}
}
/* }}} */

View file

@ -23,12 +23,13 @@ struct fpm_child_s {
struct fpm_child_s *prev, *next;
struct timeval started;
struct fpm_worker_pool_s *wp;
struct fpm_event_s ev_stdout, ev_stderr;
struct fpm_event_s ev_stdout, ev_stderr, ev_free;
int shm_slot_i;
int fd_stdout, fd_stderr;
void (*tracer)(struct fpm_child_s *);
struct timeval slow_logged;
int idle_kill;
bool idle_kill;
bool postponed_free;
pid_t pid;
int scoreboard_i;
struct zlog_stream *log_stream;

View file

@ -318,7 +318,7 @@ static void fpm_pctl_kill_idle_child(struct fpm_child_s *child) /* {{{ */
if (child->idle_kill) {
fpm_pctl_kill(child->pid, FPM_PCTL_KILL);
} else {
child->idle_kill = 1;
child->idle_kill = true;
fpm_pctl_kill(child->pid, FPM_PCTL_QUIT);
}
}

View file

@ -181,10 +181,7 @@ static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg)
if (!arg) {
return;
}
child = fpm_child_find((intptr_t) arg);
if (!child) {
return;
}
child = (struct fpm_child_s *) arg;
is_stdout = (fd == child->fd_stdout);
if (is_stdout) {
@ -277,6 +274,7 @@ stdio_read:
fpm_event_del(event);
child->postponed_free = true;
if (is_stdout) {
close(child->fd_stdout);
child->fd_stdout = -1;
@ -330,10 +328,10 @@ int fpm_stdio_parent_use_pipes(struct fpm_child_s *child) /* {{{ */
child->fd_stdout = fd_stdout[0];
child->fd_stderr = fd_stderr[0];
fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, child);
fpm_event_add(&child->ev_stdout, 0);
fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, child);
fpm_event_add(&child->ev_stderr, 0);
return 0;
}