From e3a05ecd7ac9d0353163d7c5eb702be538811d33 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sat, 2 Oct 2021 22:53:41 +0100 Subject: [PATCH 1/2] Fix bug #81026 (PHP-FPM oob R/W in root process leading to priv escalation) The main change is to store scoreboard procs directly to the variable sized array rather than indirectly through the pointer. Signed-off-by: Stanislav Malyshev (cherry picked from commit cb2021e5f69da5e2868130a05bb53db0f9f89e4b) (cherry picked from commit 4699cc1b1b957c843c71a79fa816446b622d4278) --- sapi/fpm/fpm/fpm_children.c | 14 ++--- sapi/fpm/fpm/fpm_request.c | 4 +- sapi/fpm/fpm/fpm_scoreboard.c | 107 +++++++++++++++++++-------------- sapi/fpm/fpm/fpm_scoreboard.h | 11 ++-- sapi/fpm/fpm/fpm_status.c | 4 +- sapi/fpm/fpm/fpm_worker_pool.c | 2 +- 6 files changed, 81 insertions(+), 61 deletions(-) diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c index 45cc075b42..74c2f38236 100644 --- a/sapi/fpm/fpm/fpm_children.c +++ b/sapi/fpm/fpm/fpm_children.c @@ -239,7 +239,7 @@ void fpm_children_bury() /* {{{ */ fpm_child_unlink(child); - fpm_scoreboard_proc_free(wp->scoreboard, child->scoreboard_i); + fpm_scoreboard_proc_free(child); fpm_clock_get(&tv1); @@ -249,9 +249,9 @@ void fpm_children_bury() /* {{{ */ if (!fpm_pctl_can_spawn_children()) { severity = ZLOG_DEBUG; } - zlog(severity, "[pool %s] child %d exited %s after %ld.%06d seconds from start", child->wp->config->name, (int) pid, buf, tv2.tv_sec, (int) tv2.tv_usec); + zlog(severity, "[pool %s] child %d exited %s after %ld.%06d seconds from start", wp->config->name, (int) pid, buf, tv2.tv_sec, (int) tv2.tv_usec); } else { - zlog(ZLOG_DEBUG, "[pool %s] child %d has been killed by the process management after %ld.%06d seconds from start", child->wp->config->name, (int) pid, tv2.tv_sec, (int) tv2.tv_usec); + zlog(ZLOG_DEBUG, "[pool %s] child %d has been killed by the process management after %ld.%06d seconds from start", wp->config->name, (int) pid, tv2.tv_sec, (int) tv2.tv_usec); } fpm_child_close(child, 1 /* in event_loop */); @@ -317,7 +317,7 @@ static struct fpm_child_s *fpm_resources_prepare(struct fpm_worker_pool_s *wp) / return 0; } - if (0 > fpm_scoreboard_proc_alloc(wp->scoreboard, &c->scoreboard_i)) { + if (0 > fpm_scoreboard_proc_alloc(c)) { fpm_stdio_discard_pipes(c); fpm_child_free(c); return 0; @@ -329,7 +329,7 @@ static struct fpm_child_s *fpm_resources_prepare(struct fpm_worker_pool_s *wp) / static void fpm_resources_discard(struct fpm_child_s *child) /* {{{ */ { - fpm_scoreboard_proc_free(child->wp->scoreboard, child->scoreboard_i); + fpm_scoreboard_proc_free(child); fpm_stdio_discard_pipes(child); fpm_child_free(child); } @@ -342,10 +342,10 @@ static void fpm_child_resources_use(struct fpm_child_s *child) /* {{{ */ if (wp == child->wp) { continue; } - fpm_scoreboard_free(wp->scoreboard); + fpm_scoreboard_free(wp); } - fpm_scoreboard_child_use(child->wp->scoreboard, child->scoreboard_i, getpid()); + fpm_scoreboard_child_use(child, getpid()); fpm_stdio_child_use_pipes(child); fpm_child_free(child); } diff --git a/sapi/fpm/fpm/fpm_request.c b/sapi/fpm/fpm/fpm_request.c index ed7e7a8890..23782fbb05 100644 --- a/sapi/fpm/fpm/fpm_request.c +++ b/sapi/fpm/fpm/fpm_request.c @@ -287,7 +287,7 @@ int fpm_request_is_idle(struct fpm_child_s *child) /* {{{ */ struct fpm_scoreboard_proc_s *proc; /* no need in atomicity here */ - proc = fpm_scoreboard_proc_get(child->wp->scoreboard, child->scoreboard_i); + proc = fpm_scoreboard_proc_get_from_child(child); if (!proc) { return 0; } @@ -302,7 +302,7 @@ int fpm_request_last_activity(struct fpm_child_s *child, struct timeval *tv) /* if (!tv) return -1; - proc = fpm_scoreboard_proc_get(child->wp->scoreboard, child->scoreboard_i); + proc = fpm_scoreboard_proc_get_from_child(child); if (!proc) { return -1; } diff --git a/sapi/fpm/fpm/fpm_scoreboard.c b/sapi/fpm/fpm/fpm_scoreboard.c index e1e69c9780..fcf9f16970 100644 --- a/sapi/fpm/fpm/fpm_scoreboard.c +++ b/sapi/fpm/fpm/fpm_scoreboard.c @@ -8,6 +8,7 @@ #include #include "fpm_config.h" +#include "fpm_children.h" #include "fpm_scoreboard.h" #include "fpm_shm.h" #include "fpm_sockets.h" @@ -25,7 +26,6 @@ static float fpm_scoreboard_tick; int fpm_scoreboard_init_main() /* {{{ */ { struct fpm_worker_pool_s *wp; - unsigned int i; #ifdef HAVE_TIMES #if (defined(HAVE_SYSCONF) && defined(_SC_CLK_TCK)) @@ -42,7 +42,7 @@ int fpm_scoreboard_init_main() /* {{{ */ for (wp = fpm_worker_all_pools; wp; wp = wp->next) { - size_t scoreboard_size, scoreboard_nprocs_size; + size_t scoreboard_procs_size; void *shm_mem; if (wp->config->pm_max_children < 1) { @@ -55,22 +55,15 @@ int fpm_scoreboard_init_main() /* {{{ */ return -1; } - scoreboard_size = sizeof(struct fpm_scoreboard_s) + (wp->config->pm_max_children) * sizeof(struct fpm_scoreboard_proc_s *); - scoreboard_nprocs_size = sizeof(struct fpm_scoreboard_proc_s) * wp->config->pm_max_children; - shm_mem = fpm_shm_alloc(scoreboard_size + scoreboard_nprocs_size); + scoreboard_procs_size = sizeof(struct fpm_scoreboard_proc_s) * wp->config->pm_max_children; + shm_mem = fpm_shm_alloc(sizeof(struct fpm_scoreboard_s) + scoreboard_procs_size); if (!shm_mem) { return -1; } - wp->scoreboard = shm_mem; + wp->scoreboard = shm_mem; + wp->scoreboard->pm = wp->config->pm; wp->scoreboard->nprocs = wp->config->pm_max_children; - shm_mem += scoreboard_size; - - for (i = 0; i < wp->scoreboard->nprocs; i++, shm_mem += sizeof(struct fpm_scoreboard_proc_s)) { - wp->scoreboard->procs[i] = shm_mem; - } - - wp->scoreboard->pm = wp->config->pm; wp->scoreboard->start_epoch = time(NULL); strlcpy(wp->scoreboard->pool, wp->config->name, sizeof(wp->scoreboard->pool)); } @@ -164,28 +157,47 @@ struct fpm_scoreboard_s *fpm_scoreboard_get() /* {{{*/ } /* }}} */ -struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get(struct fpm_scoreboard_s *scoreboard, int child_index) /* {{{*/ +static inline struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get_ex( + struct fpm_scoreboard_s *scoreboard, int child_index, unsigned int nprocs) /* {{{*/ { if (!scoreboard) { - scoreboard = fpm_scoreboard; + return NULL; } - if (!scoreboard) { + if (child_index < 0 || (unsigned int)child_index >= nprocs) { return NULL; } + return &scoreboard->procs[child_index]; +} +/* }}} */ + +struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get( + struct fpm_scoreboard_s *scoreboard, int child_index) /* {{{*/ +{ + if (!scoreboard) { + scoreboard = fpm_scoreboard; + } + if (child_index < 0) { child_index = fpm_scoreboard_i; } - if (child_index < 0 || child_index >= scoreboard->nprocs) { - return NULL; - } + return fpm_scoreboard_proc_get_ex(scoreboard, child_index, scoreboard->nprocs); +} + +struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get_from_child(struct fpm_child_s *child) /* {{{*/ +{ + struct fpm_worker_pool_s *wp = child->wp; + unsigned int nprocs = wp->config->pm_max_children; + struct fpm_scoreboard_s *scoreboard = wp->scoreboard; + int child_index = child->scoreboard_i; - return scoreboard->procs[child_index]; + return fpm_scoreboard_proc_get_ex(scoreboard, child_index, nprocs); } /* }}} */ + struct fpm_scoreboard_s *fpm_scoreboard_acquire(struct fpm_scoreboard_s *scoreboard, int nohang) /* {{{ */ { struct fpm_scoreboard_s *s; @@ -236,28 +248,28 @@ void fpm_scoreboard_proc_release(struct fpm_scoreboard_proc_s *proc) /* {{{ */ proc->lock = 0; } -void fpm_scoreboard_free(struct fpm_scoreboard_s *scoreboard) /* {{{ */ +void fpm_scoreboard_free(struct fpm_worker_pool_s *wp) /* {{{ */ { - size_t scoreboard_size, scoreboard_nprocs_size; + size_t scoreboard_procs_size; + struct fpm_scoreboard_s *scoreboard = wp->scoreboard; if (!scoreboard) { zlog(ZLOG_ERROR, "**scoreboard is NULL"); return; } - scoreboard_size = sizeof(struct fpm_scoreboard_s) + (scoreboard->nprocs) * sizeof(struct fpm_scoreboard_proc_s *); - scoreboard_nprocs_size = sizeof(struct fpm_scoreboard_proc_s) * scoreboard->nprocs; - - fpm_shm_free(scoreboard, scoreboard_size + scoreboard_nprocs_size); + scoreboard_procs_size = sizeof(struct fpm_scoreboard_proc_s) * wp->config->pm_max_children; + + fpm_shm_free(scoreboard, sizeof(struct fpm_scoreboard_s) + scoreboard_procs_size); } /* }}} */ -void fpm_scoreboard_child_use(struct fpm_scoreboard_s *scoreboard, int child_index, pid_t pid) /* {{{ */ +void fpm_scoreboard_child_use(struct fpm_child_s *child, pid_t pid) /* {{{ */ { struct fpm_scoreboard_proc_s *proc; - fpm_scoreboard = scoreboard; - fpm_scoreboard_i = child_index; - proc = fpm_scoreboard_proc_get(scoreboard, child_index); + fpm_scoreboard = child->wp->scoreboard; + fpm_scoreboard_i = child->scoreboard_i; + proc = fpm_scoreboard_proc_get_from_child(child); if (!proc) { return; } @@ -266,18 +278,22 @@ void fpm_scoreboard_child_use(struct fpm_scoreboard_s *scoreboard, int child_ind } /* }}} */ -void fpm_scoreboard_proc_free(struct fpm_scoreboard_s *scoreboard, int child_index) /* {{{ */ +void fpm_scoreboard_proc_free(struct fpm_child_s *child) /* {{{ */ { + struct fpm_worker_pool_s *wp = child->wp; + struct fpm_scoreboard_s *scoreboard = wp->scoreboard; + int child_index = child->scoreboard_i; + if (!scoreboard) { return; } - if (child_index < 0 || child_index >= scoreboard->nprocs) { + if (child_index < 0 || child_index >= wp->config->pm_max_children) { return; } - if (scoreboard->procs[child_index] && scoreboard->procs[child_index]->used > 0) { - memset(scoreboard->procs[child_index], 0, sizeof(struct fpm_scoreboard_proc_s)); + if (scoreboard->procs[child_index].used > 0) { + memset(&scoreboard->procs[child_index], 0, sizeof(struct fpm_scoreboard_proc_s)); } /* set this slot as free to avoid search on next alloc */ @@ -285,41 +301,44 @@ void fpm_scoreboard_proc_free(struct fpm_scoreboard_s *scoreboard, int child_ind } /* }}} */ -int fpm_scoreboard_proc_alloc(struct fpm_scoreboard_s *scoreboard, int *child_index) /* {{{ */ +int fpm_scoreboard_proc_alloc(struct fpm_child_s *child) /* {{{ */ { int i = -1; + struct fpm_worker_pool_s *wp = child->wp; + struct fpm_scoreboard_s *scoreboard = wp->scoreboard; + int nprocs = wp->config->pm_max_children; - if (!scoreboard || !child_index) { + if (!scoreboard) { return -1; } /* first try the slot which is supposed to be free */ - if (scoreboard->free_proc >= 0 && scoreboard->free_proc < scoreboard->nprocs) { - if (scoreboard->procs[scoreboard->free_proc] && !scoreboard->procs[scoreboard->free_proc]->used) { + if (scoreboard->free_proc >= 0 && scoreboard->free_proc < nprocs) { + if (!scoreboard->procs[scoreboard->free_proc].used) { i = scoreboard->free_proc; } } if (i < 0) { /* the supposed free slot is not, let's search for a free slot */ zlog(ZLOG_DEBUG, "[pool %s] the proc->free_slot was not free. Let's search", scoreboard->pool); - for (i = 0; i < scoreboard->nprocs; i++) { - if (scoreboard->procs[i] && !scoreboard->procs[i]->used) { /* found */ + for (i = 0; i < nprocs; i++) { + if (!scoreboard->procs[i].used) { /* found */ break; } } } /* no free slot */ - if (i < 0 || i >= scoreboard->nprocs) { + if (i < 0 || i >= nprocs) { zlog(ZLOG_ERROR, "[pool %s] no free scoreboard slot", scoreboard->pool); return -1; } - scoreboard->procs[i]->used = 1; - *child_index = i; + scoreboard->procs[i].used = 1; + child->scoreboard_i = i; /* supposed next slot is free */ - if (i + 1 >= scoreboard->nprocs) { + if (i + 1 >= nprocs) { scoreboard->free_proc = 0; } else { scoreboard->free_proc = i + 1; diff --git a/sapi/fpm/fpm/fpm_scoreboard.h b/sapi/fpm/fpm/fpm_scoreboard.h index f58a28737d..a0cc093b8c 100644 --- a/sapi/fpm/fpm/fpm_scoreboard.h +++ b/sapi/fpm/fpm/fpm_scoreboard.h @@ -65,7 +65,7 @@ struct fpm_scoreboard_s { unsigned int nprocs; int free_proc; unsigned long int slow_rq; - struct fpm_scoreboard_proc_s *procs[]; + struct fpm_scoreboard_proc_s procs[]; }; int fpm_scoreboard_init_main(); @@ -74,18 +74,19 @@ int fpm_scoreboard_init_child(struct fpm_worker_pool_s *wp); void fpm_scoreboard_update(int idle, int active, int lq, int lq_len, int requests, int max_children_reached, int slow_rq, int action, struct fpm_scoreboard_s *scoreboard); struct fpm_scoreboard_s *fpm_scoreboard_get(); struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get(struct fpm_scoreboard_s *scoreboard, int child_index); +struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get_from_child(struct fpm_child_s *child); struct fpm_scoreboard_s *fpm_scoreboard_acquire(struct fpm_scoreboard_s *scoreboard, int nohang); void fpm_scoreboard_release(struct fpm_scoreboard_s *scoreboard); struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_acquire(struct fpm_scoreboard_s *scoreboard, int child_index, int nohang); void fpm_scoreboard_proc_release(struct fpm_scoreboard_proc_s *proc); -void fpm_scoreboard_free(struct fpm_scoreboard_s *scoreboard); +void fpm_scoreboard_free(struct fpm_worker_pool_s *wp); -void fpm_scoreboard_child_use(struct fpm_scoreboard_s *scoreboard, int child_index, pid_t pid); +void fpm_scoreboard_child_use(struct fpm_child_s *child, pid_t pid); -void fpm_scoreboard_proc_free(struct fpm_scoreboard_s *scoreboard, int child_index); -int fpm_scoreboard_proc_alloc(struct fpm_scoreboard_s *scoreboard, int *child_index); +void fpm_scoreboard_proc_free(struct fpm_child_s *child); +int fpm_scoreboard_proc_alloc(struct fpm_child_s *child); #ifdef HAVE_TIMES float fpm_scoreboard_get_tick(); diff --git a/sapi/fpm/fpm/fpm_status.c b/sapi/fpm/fpm/fpm_status.c index 2363b57f80..a2ee398d29 100644 --- a/sapi/fpm/fpm/fpm_status.c +++ b/sapi/fpm/fpm/fpm_status.c @@ -399,10 +399,10 @@ int fpm_status_handle_request(TSRMLS_D) /* {{{ */ first = 1; for (i=0; inprocs; i++) { - if (!scoreboard_p->procs[i] || !scoreboard_p->procs[i]->used) { + if (!scoreboard_p->procs[i].used) { continue; } - proc = *scoreboard_p->procs[i]; + proc = scoreboard_p->procs[i]; if (first) { first = 0; diff --git a/sapi/fpm/fpm/fpm_worker_pool.c b/sapi/fpm/fpm/fpm_worker_pool.c index a0022915cd..c778b33c8c 100644 --- a/sapi/fpm/fpm/fpm_worker_pool.c +++ b/sapi/fpm/fpm/fpm_worker_pool.c @@ -44,7 +44,7 @@ static void fpm_worker_pool_cleanup(int which, void *arg) /* {{{ */ fpm_worker_pool_config_free(wp->config); fpm_children_free(wp->children); if ((which & FPM_CLEANUP_CHILD) == 0 && fpm_globals.parent_pid == getpid()) { - fpm_scoreboard_free(wp->scoreboard); + fpm_scoreboard_free(wp); } fpm_worker_pool_free(wp); } -- 2.31.1 From 93889e8129b8a68e41abde81ac9e7fb19fcf03a8 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Wed, 20 Oct 2021 14:11:20 +0200 Subject: [PATCH 2/2] NEWS (cherry picked from commit 28c4ee7da2cf3428113e07326dbd46550c50c2bd) (cherry picked from commit 9e0b951aee92deb470e31bd9f0e14f1434861b6a) --- NEWS | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS b/NEWS index c9cdc3a328..1b39e82475 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,12 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| +Backported from 7.4.25 + +- FPM: + . Fixed bug #81026 (PHP-FPM oob R/W in root process leading to privilege + escalation) (CVE-2021-21703). (Jakub Zelenka) + Backported from 7.3.30 - Phar: -- 2.31.1