Subject: [PATCH] Segfault in pool_do_auth() when failover races with a new connection

From: Emond Papegaaij <[email-masked]>
Date: Mon, 29 Jun 2026 10:25:51 +0200

Hi,

Last night one of CI builds detected a segmentation fault in pgpool
(4.7.2). I've ran an analysis on the coredump with Claude and came to
the conclusion that this can happen when trying to open a connection
in the middle of a failover.

Symptom / backtrace (resolved from the core dump):
pool_do_auth (src/auth/pool_auth.c:349)   <-- SIGSEGV, fault addr 0x0
connect_backend (src/protocol/child.c:1099)  (PG_TRY block)
get_backend_connection (src/protocol/child.c:2109)
do_child (src/protocol/child.c:233)

The crashing statement is:
protoMajor = MAIN_CONNECTION(cp)->sp->major;   /* pool_auth.c:349 */
i.e. cp->slots[MAIN_NODE_ID] is NULL. From the core, the pool had
slots[0] == NULL while slots[1]/slots[2] were valid, and MAIN_NODE_ID == 0.

pool_initialize_private_backend_status() snapshots
private_backend_status[] and my_main_node_id non‑atomically w.r.t. the
parent's failover, so a child can capture my_main_node_id == 0 while
node 0 is already CON_DOWN. new_connection() then never creates
slots[0], and pool_do_auth() dereferences the NULL main slot. (Same
class as the NULL‑MAIN_CONNECTION case noted in
pool_virtual_main_db_node_id().)

Attached is a patch that fixes the issue.

Best regards,
Emond
diff --git i/src/protocol/child.c w/src/protocol/child.c
index c34f05728..1f2eda81a 100644
--- i/src/protocol/child.c
+++ w/src/protocol/child.c
@@ -1476,6 +1476,29 @@ pool_initialize_private_backend_status(void)
 	}
 
 	my_main_node_id = REAL_MAIN_NODE_ID;
+
+	/*
+	 * REAL_MAIN_NODE_ID and the per-node status are read from shared memory
+	 * non-atomically, so a concurrent failover can leave my_main_node_id
+	 * pointing at a node we just captured as down. Re-point it at the first
+	 * node that is up in our private snapshot; otherwise MAIN_CONNECTION()
+	 * would dereference a connection slot that is never created for a down
+	 * node.
+	 */
+	if (my_main_node_id < 0 || my_main_node_id >= NUM_BACKENDS ||
+		(private_backend_status[my_main_node_id] != CON_UP &&
+		 private_backend_status[my_main_node_id] != CON_CONNECT_WAIT))
+	{
+		for (i = 0; i < NUM_BACKENDS; i++)
+		{
+			if (private_backend_status[i] == CON_UP ||
+				private_backend_status[i] == CON_CONNECT_WAIT)
+			{
+				my_main_node_id = i;
+				break;
+			}
+		}
+	}
 }
 
 static void
diff --git i/src/protocol/pool_connection_pool.c w/src/protocol/pool_connection_pool.c
index b16ccc39e..a939b08a7 100644
--- i/src/protocol/pool_connection_pool.c
+++ w/src/protocol/pool_connection_pool.c
@@ -1035,6 +1035,26 @@ new_connection(POOL_CONNECTION_POOL *p)
 
 	if (active_backend_count > 0)
 	{
+		/*
+		 * A concurrent failover may have changed backend status while we were
+		 * creating connections, leaving my_main_node_id pointing at a node we
+		 * skipped (its slot is NULL). Re-point it at a node we did connect to
+		 * so that MAIN_CONNECTION() never dereferences a NULL slot (e.g. in
+		 * pool_do_auth()).
+		 */
+		if (my_main_node_id < 0 || my_main_node_id >= NUM_BACKENDS ||
+			p->slots[my_main_node_id] == NULL)
+		{
+			for (i = 0; i < NUM_BACKENDS; i++)
+			{
+				if (p->slots[i] != NULL)
+				{
+					my_main_node_id = i;
+					break;
+				}
+			}
+		}
+
 		return p;
 	}

---

From: Tatsuo Ishii <[email-masked]>
Date: Tue, 30 Jun 2026 16:46:00 +0900 (JST)

Hi Emonda,

> Attached is a patch that fixes the issue.

Thanks for the patch. I will look into it.

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

---

From: Tatsuo Ishii <[email-masked]>
Date: Thu, 02 Jul 2026 16:39:22 +0900 (JST)

Hi Emond,

> pool_virtual_main_db_node_id().)

From pool_initialize_private_backend_status():

	for (i = 0; i < MAX_NUM_BACKENDS; i++)
	{
		private_backend_status[i] = BACKEND_INFO(i).backend_status;
		/* my_backend_status is referred to by VALID_BACKEND macro. */
		my_backend_status[i] = &private_backend_status[i];
	}

BACKEND_INFO(i).backend_status are updated in the pgpool main process
at line 1793 of failover(pgpool_main.c) by calling
handle_failover_request(), but REAL_MAIN_NODE_ID (actually
Req_info->main_node_id, shared memory) is updated at 1846 of
failover() by calling save_node_info(). So updating REAL_MAIN_NODE_ID
tends to slightly delay after updating
BACKEND_INFO(i).backend_status. This may explain why my_main_node is
set the stale data (1).

Another possibility is my_main_node_id being lacking volatile
qualification. This also could set stale data to my_main_node_id (2).

Of course (1) and (2) are not mutually exclusive. So your patch for
(1) and fix for (2) need to be both applied.

By the way in your patch there is loops like this. This can be
replaced by existing get_next_main_node().

All of the above in mind, I propose attached patch.

What do you think?
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
diff --git a/src/include/pool.h b/src/include/pool.h
index 549aed30f..7ead2e926 100644
--- a/src/include/pool.h
+++ b/src/include/pool.h
@@ -345,8 +345,8 @@ extern int	pool_get_major_version(void);
 extern bool pool_is_node_to_be_sent_in_current_query(int node_id);
 extern int	pool_virtual_main_db_node_id(void);
 
-extern BACKEND_STATUS *my_backend_status[];
-extern int	my_main_node_id;
+extern volatile BACKEND_STATUS *my_backend_status[];
+extern volatile int	my_main_node_id;
 
 #define VALID_BACKEND(backend_id) \
 	((RAW_MODE && (backend_id) == REAL_MAIN_NODE_ID) ||		\
@@ -662,7 +662,7 @@ extern volatile sig_atomic_t ignore_sigusr1;
 #define QUERY_STRING_BUFFER_LEN 1024
 extern char query_string_buffer[];	/* last query string sent to simpleQuery() */
 
-extern BACKEND_STATUS private_backend_status[MAX_NUM_BACKENDS];
+extern volatile BACKEND_STATUS private_backend_status[MAX_NUM_BACKENDS];
 
 extern char remote_host[];		/* client host */
 extern char remote_port[];		/* client port */
diff --git a/src/main/pgpool_main.c b/src/main/pgpool_main.c
index 19d89cb79..a99b1b5b5 100644
--- a/src/main/pgpool_main.c
+++ b/src/main/pgpool_main.c
@@ -226,7 +226,7 @@ static pid_t health_check_pids[MAX_NUM_BACKENDS];
 /*
  * Private copy of backend status
  */
-BACKEND_STATUS private_backend_status[MAX_NUM_BACKENDS];
+volatile BACKEND_STATUS private_backend_status[MAX_NUM_BACKENDS];
 
 /*
  * shmem connection info table
@@ -278,7 +278,7 @@ static pid_t wd_lifecheck_pid = 0;	/* pid for child process handling watchdog
 									 * lifecheck */
 
 BACKEND_STATUS *my_backend_status[MAX_NUM_BACKENDS];	/* Backend status buffer */
-int			my_main_node_id;	/* Main node id buffer */
+volatile int			my_main_node_id;	/* Main node id buffer */
 
 /*
  * Dummy variable to suppress compiler warnings by discarding return values
diff --git a/src/protocol/child.c b/src/protocol/child.c
index 4a527c84c..67a3c6c9a 100644
--- a/src/protocol/child.c
+++ b/src/protocol/child.c
@@ -1483,7 +1483,20 @@ pool_initialize_private_backend_status(void)
 		my_backend_status[i] = &private_backend_status[i];
 	}
 
-	my_main_node_id = REAL_MAIN_NODE_ID;
+ 	my_main_node_id = REAL_MAIN_NODE_ID;
+
+	/*
+	 * REAL_MAIN_NODE_ID and the per-node status are read from shared memory
+	 * non-atomically, so a concurrent failover can leave my_main_node_id
+	 * pointing at a node we just captured as down. Re-point it at the first
+	 * node that is up in our private snapshot; otherwise MAIN_CONNECTION()
+	 * would dereference a connection slot that is never created for a down
+	 * node.
+	 */
+	if (my_main_node_id < 0 || my_main_node_id >= NUM_BACKENDS ||
+		(private_backend_status[my_main_node_id] != CON_UP &&
+		 private_backend_status[my_main_node_id] != CON_CONNECT_WAIT))
+		my_main_node_id = get_next_main_node();
 }
 
 static void
diff --git a/src/protocol/pool_connection_pool.c b/src/protocol/pool_connection_pool.c
index b16ccc39e..a939b08a7 100644
--- a/src/protocol/pool_connection_pool.c
+++ b/src/protocol/pool_connection_pool.c
@@ -1035,6 +1035,26 @@ new_connection(POOL_CONNECTION_POOL *p)

---

From: Emond Papegaaij <[email-masked]>
Date: Thu, 2 Jul 2026 11:21:13 +0200

Hi,

The new patch looks fine to me. There's however one thing Claude
pointed out. According to it, my_main_node_id,
private_backend_status[], and my_backend_status[] are per-process
globals (copied on fork), not shared memory. So, making any of those
volatile should not make any difference.

Best regards,
Emond

---

From: Tatsuo Ishii <[email-masked]>
Date: Thu, 02 Jul 2026 22:45:17 +0900 (JST)

Hi Emond,

> volatile should not make any difference.

You are right. Attached is the v3 patch.

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
diff --git a/src/protocol/child.c b/src/protocol/child.c
index 4a527c84c..a4ec13dfd 100644
--- a/src/protocol/child.c
+++ b/src/protocol/child.c
@@ -1478,12 +1478,26 @@ pool_initialize_private_backend_status(void)
 
 	for (i = 0; i < MAX_NUM_BACKENDS; i++)
 	{
-		private_backend_status[i] = BACKEND_INFO(i).backend_status;
+		private_backend_status[i] =
+			*(volatile BACKEND_STATUS *) &BACKEND_INFO(i).backend_status;
 		/* my_backend_status is referred to by VALID_BACKEND macro. */
 		my_backend_status[i] = &private_backend_status[i];
 	}
 
+ 	my_main_node_id = *(volatile int *) &REAL_MAIN_NODE_ID;
 
 static void

---

From: Emond Papegaaij <[email-masked]>
Date: Thu, 2 Jul 2026 15:54:56 +0200

Hi Tatsuo,

Yes, this looks good. There's one small nit: there's some whitespace
noise in the patch (space before tab) on the line with
"my_main_node_id = *(volatile int *) &REAL_MAIN_NODE_ID;"

Best regards,
Emond

---

From: Tatsuo Ishii <[email-masked]>
Date: Fri, 03 Jul 2026 15:31:48 +0900 (JST)

Hi Emond,

> "my_main_node_id = *(volatile int *) &REAL_MAIN_NODE_ID;"

I fixed this and pushed to all supported branches.
https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=c39c7b40bf7573e8fc1b4772e99c3b371dca10ed

Thank you!

---

From: Emond Papegaaij <[email-masked]>
Date: Fri, 3 Jul 2026 09:22:34 +0200

Hi Tatsuo,

Thank you again for taking care of all this!

Emond