Fix GH-19974: segmentation fault when call fpm_get_status

[txuna]

Close #19974
This issue occurs because the used field is set incorrectly during the process list pointer reference

// bug.php
<?php
fpm_get_status();
?>

Image two child processes A and B.

pm.max_children = 2;

When a client request like below

curl localhost/bug.php

Process A is assumed to have acquired the process lock and to be running, as shown in the code below.

void fpm_request_finished(void)
{
	struct fpm_scoreboard_proc_s *proc;
	struct timeval now;

	fpm_clock_get(&now);

	proc = fpm_scoreboard_proc_acquire(NULL, -1, 0);
	if (proc == NULL) {
		zlog(ZLOG_WARNING, "failed to acquire proc scoreboard");
		return;
	}

	proc->request_stage = FPM_REQUEST_FINISHED;    <------ ** Process(A) EIP(instruction pointer)**
	proc->tv = now;
	fpm_scoreboard_proc_release(proc);
}

At this point, if the client sends another request, it will be handled by Process B, and the corresponding code is shown below.

int fpm_status_export_to_zval(zval *status)
{
[...]
	scoreboard = *scoreboard_p;
	struct fpm_scoreboard_proc_s *procs = safe_emalloc(
			sizeof(struct fpm_scoreboard_proc_s), scoreboard.nprocs, 0);

	struct fpm_scoreboard_proc_s *proc_p;
	for(i=0; i<scoreboard.nprocs; i++) {
		proc_p = fpm_scoreboard_proc_acquire(scoreboard_p, i, 1);  <------- ** Process B Here! **
		if (!proc_p){
			procs[i].used=-1;
			continue;
		}
		procs[i] = *proc_p;
		fpm_scoreboard_proc_release(proc_p);
	}

Naturally, since Process A currently holds the lock, Process B cannot obtain the proc pointer. so proc_p is NULL
However, it sets the value of the used field not to 0, but to -1.

	for(i=0; i<scoreboard.nprocs; i++) {
		if (!procs[i].used) {
			continue;
		}
		proc_p = &procs[i];
[...]
-----------> **Segfault this line**
		add_assoc_string(&fpm_proc_stat, "state", fpm_request_get_stage_name(procs[i].request_stage));

Note

The most important point is that -1 always evaluates to true.

Therefore, the procs array fails to retrieve the pointer to the locked Process A and instead sets used to -1. Since -1 evaluates to true, subsequent access proceeds and the request_stage field ends up referencing an uninitialized garbage value, which can cause an error when fpm_request_get_stage_name is called.

POC

pm.max_children = 2;

client

curl localhost/bug.php  # THIS IS OK

curl localhost/bug.php # Segmentation Fault!

gdb A process

Acquire the lock, then execute the critical section until the lock is released.

gdb -p [A PID]
b fpm_request_finished
c

gdb B process

gdb -p [A PID]
b fpm_status_export_to_zval
c

In file: /home/tuuna.linux/research/php-src/sapi/fpm/fpm/fpm_status.c:72
   67
   68         struct fpm_scoreboard_proc_s *proc_p;
   69         for(i=0; i<scoreboard.nprocs; i++) {
   70                 proc_p = fpm_scoreboard_proc_acquire(scoreboard_p, i, 1);
   71                 if (!proc_p){
 ► 72                         procs[i].used=-1;
   73                         continue;
   74                 }
   75                 procs[i] = *proc_p;
   76                 fpm_scoreboard_proc_release(proc_p);
   77         }
───────────[ BACKTRACE ]──────────────
 ► 0   0xc5fad43e579c fpm_status_export_to_zval+268
   1   0xc5fad43dd4fc zif_fpm_get_status+136
   2   0xc5fad426857c ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER+168
   3   0xc5fad42f7140 execute_ex+2292
   4   0xc5fad42fb958 zend_execute+296
   5   0xc5fad421cf9c zend_execute_scripts+356
   6   0xc5fad415a6e8 php_execute_script+648
   7   0xc5fad43de13c main+3108
────────────────────────────────
pwndbg> p proc_p
$25 = (struct fpm_scoreboard_proc_s *) 0x0
pwndbg> p i
$26 = 0
pwndbg> p scoreboard.nprocs
$27 = 2

In file: /home/tuuna.linux/research/php-src/sapi/fpm/fpm/fpm_status.c:101
    96         add_assoc_long(status, "max-children-reached", scoreboard.max_children_reached);
    97         add_assoc_long(status, "slow-requests", scoreboard.slow_rq);
    98
    99         array_init(&fpm_proc_stats);
   100         for(i=0; i<scoreboard.nprocs; i++) {
 ► 101                 if (!procs[i].used) {
   102                         continue;
   103                 }
   104                 proc_p = &procs[i];
   105                 /* prevent NaN */
   106                 if (procs[i].cpu_duration.tv_sec == 0 && procs[i].cpu_duration.tv_usec == 0) {
─────────[ BACKTRACE ]──────────────
 ► 0   0xc5fad43e5a6c fpm_status_export_to_zval+988
   1   0xc5fad43dd4fc zif_fpm_get_status+136
   2   0xc5fad426857c ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER+168
   3   0xc5fad42f7140 execute_ex+2292
   4   0xc5fad42fb958 zend_execute+296
   5   0xc5fad421cf9c zend_execute_scripts+356
   6   0xc5fad415a6e8 php_execute_script+648
   7   0xc5fad43de13c main+3108
───────────────────────────────
pwndbg> p procs[i].used
$28 = -1
pwndbg> p procs[i].request_stage
$29 = 4294967295

you can check request_stage is uninitialized value!

How To FIX

There are two ways to resolve this issue:

1. When the proc pointer cannot be retrieved, set the used field value to 0 instead of -1.

2. Modify the code to ensure that the proc pointer can always be retrieved by changing fpm_scoreboard_proc_acquire(scoreboard_p, i, 1) to fpm_scoreboard_proc_acquire(scoreboard_p, i, 0).

I would appreciate it if the maintainer could decide which approach to take.