Skip to content

Commit

Permalink
Fix bug #81026 (PHP-FPM oob R/W in root process leading to priv escal…
Browse files Browse the repository at this point in the history
…ation)

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>
(cherry picked from commit cb2021e)

Closes GH-7614.
  • Loading branch information
bukka authored and cmb69 committed Oct 26, 2021
1 parent 32c0850 commit f47798e
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 61 deletions.
8 changes: 7 additions & 1 deletion NEWS
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
PHP NEWS
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
?? ??? ????, PHP 7.3.32
?? ??? ????, PHP 7.3.33


28 Okt 2021, PHP 7.3.32

This comment has been minimized.

Copy link
@thg2k

thg2k Oct 27, 2021

Contributor

Okt - Oct

This comment has been minimized.

Copy link
@cmb69

cmb69 Oct 27, 2021

Contributor

Already fixed as ea16d7b.


- FPM:
. Fixed bug #81026 (PHP-FPM oob R/W in root process leading to privilege
escalation). (CVE-2021-21703) (Jakub Zelenka)

23 Sep 2021, PHP 7.3.31

- Zip:
Expand Down
14 changes: 7 additions & 7 deletions sapi/fpm/fpm/fpm_children.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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 */);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions sapi/fpm/fpm/fpm_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
106 changes: 63 additions & 43 deletions sapi/fpm/fpm/fpm_scoreboard.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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))
Expand All @@ -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) {
Expand All @@ -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));
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -264,60 +277,67 @@ 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 */
scoreboard->free_proc = child_index;
}
/* }}} */

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;
Expand Down
11 changes: 6 additions & 5 deletions sapi/fpm/fpm/fpm_scoreboard.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions sapi/fpm/fpm/fpm_status.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion sapi/fpm/fpm/fpm_worker_pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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);
}
Expand Down

0 comments on commit f47798e

Please sign in to comment.