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 06:46:33 +0000 (15:46 +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 3310530a0c42d48803bdcab20dd7ffacf6fa9bf8..268c1748fe57b3e7f44333a38195c7f00d7fa624 100644 (file)
@@ -1476,16 +1476,16 @@ 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++)
        {
                int exist_live_connection = 0;
 
-               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++)
                {
@@ -1656,6 +1656,9 @@ pools_reporting(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend)
        pfree(pools);
 }
 
+/*
+ * Used by SHOW pool_processes
+ */
 POOL_REPORT_PROCESSES *
 get_processes(int *nrows)
 {
@@ -1665,14 +1668,14 @@ 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++)
        {
                int exist_live_connection = 0;
 
-               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++)
                {
@@ -1714,7 +1717,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);