From c9c1934ff0ac1661efee56754678d0fdb073228f Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 28 Aug 2022 14:58:42 +0100 Subject: [PATCH] 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. --- NEWS | 2 ++ sapi/fpm/fpm/fpm_children.c | 2 +- sapi/fpm/fpm/fpm_children.h | 5 +++-- sapi/fpm/fpm/fpm_stdio.c | 9 ++++++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index c0faaf113d9..41177584948 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,8 @@ PHP NEWS #66694). (Petr Sumbera) . Fixed bug #68207 (Setting fastcgi.error_header can result in a WARNING). (Jakub Zelenka) + . Fixed bug GH-8517 (Random crash of FPM master process in + fpm_stdio_child_said). (Jakub Zelenka) - MBString: . Fixed bug GH-9535 (The behavior of mb_strcut in mbstring has been changed in diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c index 741fc276315..6a5787c7ec8 100644 --- a/sapi/fpm/fpm/fpm_children.c +++ b/sapi/fpm/fpm/fpm_children.c @@ -120,7 +120,7 @@ static void fpm_child_unlink(struct fpm_child_s *child) /* {{{ */ } /* }}} */ -static struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */ +struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */ { struct fpm_worker_pool_s *wp; struct fpm_child_s *child = 0; diff --git a/sapi/fpm/fpm/fpm_children.h b/sapi/fpm/fpm/fpm_children.h index 5ae4e2062e4..679c34ba038 100644 --- a/sapi/fpm/fpm/fpm_children.h +++ b/sapi/fpm/fpm/fpm_children.h @@ -10,13 +10,14 @@ #include "fpm_events.h" #include "zlog.h" +struct fpm_child_s; + int fpm_children_create_initial(struct fpm_worker_pool_s *wp); int fpm_children_free(struct fpm_child_s *child); void fpm_children_bury(void); int fpm_children_init_main(void); int fpm_children_make(struct fpm_worker_pool_s *wp, int in_event_loop, int nb_to_spawn, int is_debug); - -struct fpm_child_s; +struct fpm_child_s *fpm_child_find(pid_t pid); struct fpm_child_s { struct fpm_child_s *prev, *next; diff --git a/sapi/fpm/fpm/fpm_stdio.c b/sapi/fpm/fpm/fpm_stdio.c index d9f569a6fa9..78326924acd 100644 --- a/sapi/fpm/fpm/fpm_stdio.c +++ b/sapi/fpm/fpm/fpm_stdio.c @@ -181,7 +181,10 @@ static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg) if (!arg) { return; } - child = (struct fpm_child_s *)arg; + child = fpm_child_find((intptr_t) arg); + if (!child) { + return; + } is_stdout = (fd == child->fd_stdout); if (is_stdout) { @@ -327,10 +330,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, child); + fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid); fpm_event_add(&child->ev_stdout, 0); - fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, child); + fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid); fpm_event_add(&child->ev_stderr, 0); return 0; }