public inbox for [email protected]  

help / color / mirror / Atom feed

From: Emond Papegaaij <[email protected]>
To: [email protected]
Subject: [PATCH] Segfault in pool_do_auth() when failover races with a new connection
Date: Mon, 29 Jun 2026 10:25:51 +0200
Message-ID: <CAGXsc+Y7OKp93zSqt1WUCh7ABC7kyZqrxJPFA8AC1woB30S8EQ@mail.gmail.com> (raw)

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


Attachments:

  [text/x-patch] connect_crash_fix.patch (1.9K, ../CAGXsc+Y7OKp93zSqt1WUCh7ABC7kyZqrxJPFA8AC1woB30S8EQ@mail.gmail.com/2-connect_crash_fix.patch)
  download | inline diff:
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;
 	}
 

view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: [PATCH] Segfault in pool_do_auth() when failover races with a new connection
  In-Reply-To: <CAGXsc+Y7OKp93zSqt1WUCh7ABC7kyZqrxJPFA8AC1woB30S8EQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox