From 81bf9b1a9f6def4a6f742a6b41ddc92005ab638f Mon Sep 17 00:00:00 2001

From: Jakub Zelenka <bukka@php.net>

Date: Sat, 2 Oct 2021 22:53:41 +0100

Subject: [PATCH] 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 <stas@php.net>

---

 sapi/fpm/fpm/fpm_children.c    |  14 ++---

 sapi/fpm/fpm/fpm_request.c     |   4 +-

 sapi/fpm/fpm/fpm_scoreboard.c  | 106 ++++++++++++++++++++-------------

 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(+), 60 deletions(-)

diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c

index fd121372f37c..912f77c11aa7 100644

--- a/sapi/fpm/fpm/fpm_children.c

+++ b/sapi/fpm/fpm/fpm_children.c

@@ -246,7 +246,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);

@@ -256,9 +256,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 */);

@@ -324,7 +324,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;

@@ -336,7 +336,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);

 }

@@ -349,10 +349,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 c80aa144628f..0a6f6a7cfbf0 100644

--- a/sapi/fpm/fpm/fpm_request.c

+++ b/sapi/fpm/fpm/fpm_request.c

@@ -285,7 +285,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;

 	}

@@ -300,7 +300,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 328f999f0c9b..7e9da4d6848a 100644

--- a/sapi/fpm/fpm/fpm_scoreboard.c

+++ b/sapi/fpm/fpm/fpm_scoreboard.c

@@ -6,6 +6,7 @@

 #include <time.h>

 #include "fpm_config.h"

+#include "fpm_children.h"

 #include "fpm_scoreboard.h"

 #include "fpm_shm.h"

 #include "fpm_sockets.h"

@@ -23,7 +24,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))

@@ -40,7 +40,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) {

@@ -53,22 +53,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));

 	}

@@ -162,28 +155,48 @@ 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 || (unsigned int)child_index >= scoreboard->nprocs) {

-		return NULL;

-	}

+	return fpm_scoreboard_proc_get_ex(scoreboard, child_index, scoreboard->nprocs);

+}

+/* }}} */

-	return scoreboard->procs[child_index];

+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 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;

@@ -234,28 +247,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;

+	scoreboard_procs_size = sizeof(struct fpm_scoreboard_proc_s) * wp->config->pm_max_children;

-	fpm_shm_free(scoreboard, scoreboard_size + scoreboard_nprocs_size);

+	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;

 	}

@@ -264,18 +277,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 || (unsigned int)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 */

@@ -283,41 +300,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 && (unsigned int)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 < (int)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 >= (int)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 >= (int)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 1fecde1d0feb..9d5981e1c739 100644

--- a/sapi/fpm/fpm/fpm_scoreboard.h

+++ b/sapi/fpm/fpm/fpm_scoreboard.h

@@ -63,7 +63,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();

@@ -72,18 +72,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 36d224063583..de8db9d61a25 100644

--- a/sapi/fpm/fpm/fpm_status.c

+++ b/sapi/fpm/fpm/fpm_status.c

@@ -498,10 +498,10 @@ int fpm_status_handle_request(void) /* {{{ */

 			first = 1;

 			for (i=0; i<scoreboard_p->nprocs; 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 d04528f4e0d0..65a9b226b1ae 100644

--- a/sapi/fpm/fpm/fpm_worker_pool.c

+++ b/sapi/fpm/fpm/fpm_worker_pool.c

@@ -54,7 +54,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);

 	}