Fix use of variable after pfree

Started by Shlok Kyal4 months ago3 messages
#1Shlok Kyal
shlok.kyal.oss@gmail.com
1 attachment(s)

Hi,

While going through the code of the slot sync worker, I found that in
functions ReplSlotSyncWorkerMain and pg_sync_replication_slots the
variable app_name.data is being used after it is freed.

We can get logs as following:
2025-09-02 12:26:48.520 IST [3908359] ERROR: synchronization worker
"" could not connect to the primary server: connection to server at
"localhost" (127.0.0.1), port 5432 failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?

I have moved the pfree(app_data.name) after its usage.

This change was introduced in PG_18.
The patch applies in the HEAD and REL_18_STABLE branches.

Thanks,
Shlok Kyal

Attachments:

v1-0001-Fix-use-of-variable-after-pfree.patchapplication/octet-stream; name=v1-0001-Fix-use-of-variable-after-pfree.patchDownload
From 2b866daaa4fb1076f1bf6beea7887dedfb01d607 Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Tue, 2 Sep 2025 12:30:59 +0530
Subject: [PATCH v1] Fix use of variable after pfree

In functions ReplSlotSyncWorkerMain and pg_sync_replication_slots the
variable app_name.data is being used after it is freed. This commit fix
this behaviour.
---
 src/backend/replication/logical/slotsync.c | 3 ++-
 src/backend/replication/slotfuncs.c        | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index f5501c106dc..9d0072a49ed 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1476,7 +1476,6 @@ ReplSlotSyncWorkerMain(const void *startup_data, size_t startup_data_len)
 	 */
 	wrconn = walrcv_connect(PrimaryConnInfo, false, false, false,
 							app_name.data, &err);
-	pfree(app_name.data);
 
 	if (!wrconn)
 		ereport(ERROR,
@@ -1484,6 +1483,8 @@ ReplSlotSyncWorkerMain(const void *startup_data, size_t startup_data_len)
 				errmsg("synchronization worker \"%s\" could not connect to the primary server: %s",
 					   app_name.data, err));
 
+	pfree(app_name.data);
+
 	/*
 	 * Register the disconnection callback.
 	 *
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 69f4c6157c5..b8f21153e7b 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -921,7 +921,6 @@ pg_sync_replication_slots(PG_FUNCTION_ARGS)
 	/* Connect to the primary server. */
 	wrconn = walrcv_connect(PrimaryConnInfo, false, false, false,
 							app_name.data, &err);
-	pfree(app_name.data);
 
 	if (!wrconn)
 		ereport(ERROR,
@@ -929,6 +928,8 @@ pg_sync_replication_slots(PG_FUNCTION_ARGS)
 				errmsg("synchronization worker \"%s\" could not connect to the primary server: %s",
 					   app_name.data, err));
 
+	pfree(app_name.data);
+
 	SyncReplicationSlots(wrconn);
 
 	walrcv_disconnect(wrconn);
-- 
2.34.1

#2Amit Kapila
amit.kapila16@gmail.com
In reply to: Shlok Kyal (#1)
Re: Fix use of variable after pfree

On Tue, Sep 2, 2025 at 1:02 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:

Hi,

While going through the code of the slot sync worker, I found that in
functions ReplSlotSyncWorkerMain and pg_sync_replication_slots the
variable app_name.data is being used after it is freed.

We can get logs as following:
2025-09-02 12:26:48.520 IST [3908359] ERROR: synchronization worker
"" could not connect to the primary server: connection to server at
"localhost" (127.0.0.1), port 5432 failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?

I have moved the pfree(app_data.name) after its usage.

This change was introduced in PG_18.
The patch applies in the HEAD and REL_18_STABLE branches.

Thanks for the patch. It looks good to me. I'll take care of it.

--
With Regards,
Amit Kapila.

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Amit Kapila (#2)
Re: Fix use of variable after pfree

On 2 Sep 2025, at 09:42, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Tue, Sep 2, 2025 at 1:02 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:

While going through the code of the slot sync worker, I found that in
functions ReplSlotSyncWorkerMain and pg_sync_replication_slots the
variable app_name.data is being used after it is freed.

We can get logs as following:
2025-09-02 12:26:48.520 IST [3908359] ERROR: synchronization worker
"" could not connect to the primary server: connection to server at
"localhost" (127.0.0.1), port 5432 failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?

I have moved the pfree(app_data.name) after its usage.

This change was introduced in PG_18.
The patch applies in the HEAD and REL_18_STABLE branches.

Thanks for the patch. It looks good to me. I'll take care of it.

Agreed, this looks correct.

--
Daniel Gustafsson