Fix rare segfaults in pcp_proc_info, SHOW pool_pools and SHOW pool_processes.
authorTatsuo Ishii <ishii@sraoss.co.jp>
Fri, 23 Sep 2022 06:46:33 +0000 (15:46 +0900)
committerTatsuo Ishii <ishii@sraoss.co.jp>
Fri, 23 Sep 2022 07:42:13 +0000 (16:42 +0900)
The segfaults were in get_pools() and get_processes(). They first
extracted pid of particular process info slot on shared memory then
searched the slot again by using pid as the key. Because these steps
were not protected by any locking, it was possible that the search
using the pid failed and returned NULL if the process id is
overwritten by pgpool parent which is responsible for forking new
child process after the process exiting. As a result any subsequent
reference to the NULL pointer generated segfaults.

Solution is, first get the pointer to the process info slot then
extract the process id member from the pointer. This way, still
concurrent updating to the shared memory info by the parent process is
possible (which may lead to strange results in the output) but at
least we can avoid segfaults.

src/utils/pool_process_reporting.c

index 31669648a2945249a1b78b876122b1d1fda05bb4..7726f6f964f53e793be5039b2757bffe9c6cc8c6 100644 (file)
@@ -1388,14 +1388,14 @@ get_pools(int *nrows)
 
        int                     lines = 0;
 
-       POOL_REPORT_POOLS *pools = palloc(
-                                                                         pool_config->num_init_children * pool_config->max_pool * NUM_BACKENDS * sizeof(POOL_REPORT_POOLS)
+       POOL_REPORT_POOLS *pools = palloc0(
+               pool_config->num_init_children * pool_config->max_pool * NUM_BACKENDS * sizeof(POOL_REPORT_POOLS)
        );
 
        for (child = 0; child < pool_config->num_init_children; child++)
        {
-               proc_id = process_info[child].pid;
-               pi = pool_get_process_info(proc_id);
+               pi = &process_info[child];
+               proc_id = pi->pid;
 
                for (pool = 0; pool < pool_config->max_pool; pool++)
                {
@@ -1582,6 +1582,9 @@ pools_reporting(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend)
        pfree(pools);
 }
 
+/*
+ * Used by SHOW pool_processes
+ */
 POOL_REPORT_PROCESSES *
 get_processes(int *nrows)
 {
@@ -1591,12 +1594,12 @@ get_processes(int *nrows)
        ProcessInfo *pi = NULL;
        int                     proc_id;
 
-       POOL_REPORT_PROCESSES *processes = palloc(pool_config->num_init_children * sizeof(POOL_REPORT_PROCESSES));
+       POOL_REPORT_PROCESSES *processes = palloc0(pool_config->num_init_children * sizeof(POOL_REPORT_PROCESSES));
 
        for (child = 0; child < pool_config->num_init_children; child++)
        {
-               proc_id = process_info[child].pid;
-               pi = pool_get_process_info(proc_id);
+               pi = &process_info[child];
+               proc_id = pi->pid;
 
                snprintf(processes[child].pool_pid, POOLCONFIG_MAXCOUNTLEN, "%d", proc_id);
                strftime(processes[child].start_time, POOLCONFIG_MAXDATELEN, "%Y-%m-%d %H:%M:%S", localtime(&pi->start_time));
@@ -1608,7 +1611,9 @@ get_processes(int *nrows)
                for (pool = 0; pool < pool_config->max_pool; pool++)
                {
                        poolBE = pool * MAX_NUM_BACKENDS;
-                       if (pi->connection_info[poolBE].connected && strlen(pi->connection_info[poolBE].database) > 0 && strlen(pi->connection_info[poolBE].user) > 0)
+                       if (pi->connection_info[poolBE].connected &&
+                               strlen(pi->connection_info[poolBE].database) > 0 &&
+                               strlen(pi->connection_info[poolBE].user) > 0)
                        {
                                StrNCpy(processes[child].database, pi->connection_info[poolBE].database, POOLCONFIG_MAXIDENTLEN);
                                StrNCpy(processes[child].username, pi->connection_info[poolBE].user, POOLCONFIG_MAXIDENTLEN);