v2-0001-Fix-guc_malloc-calls-consistency-and-OOM-checks.patch

From 1cdd367823d5d2ecb8d097e707f7408757db89d4 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Fri, 14 Mar 2025 22:56:58 +0100
Subject: [PATCH v2] Fix guc_malloc calls consistency and OOM checks

check_createrole_self_grant and check_synchronized_standby_slots
were allocating memory on a LOG elevel without checking if the
allocation succeeded or not, which would have led to a segfault
if it didn't.

On top of that, quite calls were using the ERROR level relying
on erroring out rather than returning false and leting the GUC
machinery handle it gracefully.  Some other calls used WARNING
instead of LOG.  While not wrong, let's make sure all check_
functions do it consistently.

init_custom_variable gets a promoted elevel to FATAL to keep
the guc_malloc error handling in line with the rest of the
error handling in that function which already call FATAL. If
we encounter an OOM here there is no graceful handling to be
had, better to error our hard.

Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Nikita <pm91.arapov@gmail.com>
Bug: #18845
Discussion: https://postgr.es/m/18845-582c6e10247377ec@postgresql.org
---
 src/backend/access/transam/xlog.c         |  4 +++-
 src/backend/access/transam/xlogrecovery.c | 12 +++++++++---
 src/backend/commands/user.c               |  2 ++
 src/backend/commands/variable.c           |  4 ++--
 src/backend/replication/slot.c            |  2 ++
 src/backend/storage/file/fd.c             |  8 +++++++-
 src/backend/tcop/backend_startup.c        |  4 +++-
 src/backend/tcop/postgres.c               |  4 +++-
 src/backend/utils/error/elog.c            |  8 ++++++--
 src/backend/utils/misc/guc.c              |  4 ++--
 10 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 799fc739e18..0c4c5b9a368 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4791,7 +4791,9 @@ check_wal_consistency_checking(char **newval, void **extra, GucSource source)
 	list_free(elemlist);
 
 	/* assign new value */
-	*extra = guc_malloc(ERROR, (RM_MAX_ID + 1) * sizeof(bool));
+	*extra = guc_malloc(LOG, (RM_MAX_ID + 1) * sizeof(bool));
+	if (!*extra)
+		return false;
 	memcpy(*extra, newwalconsistency, (RM_MAX_ID + 1) * sizeof(bool));
 	return true;
 }
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 2c19013c98b..0aa3ab59085 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -4833,7 +4833,9 @@ check_recovery_target_lsn(char **newval, void **extra, GucSource source)
 		if (have_error)
 			return false;
 
-		myextra = (XLogRecPtr *) guc_malloc(ERROR, sizeof(XLogRecPtr));
+		myextra = (XLogRecPtr *) guc_malloc(LOG, sizeof(XLogRecPtr));
+		if (!myextra)
+			return false;
 		*myextra = lsn;
 		*extra = myextra;
 	}
@@ -4997,7 +4999,9 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source)
 		}
 	}
 
-	myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(ERROR, sizeof(RecoveryTargetTimeLineGoal));
+	myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(LOG, sizeof(RecoveryTargetTimeLineGoal));
+	if (!myextra)
+		return false;
 	*myextra = rttg;
 	*extra = myextra;
 
@@ -5033,7 +5037,9 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
 		if (errno == EINVAL || errno == ERANGE)
 			return false;
 
-		myextra = (TransactionId *) guc_malloc(ERROR, sizeof(TransactionId));
+		myextra = (TransactionId *) guc_malloc(LOG, sizeof(TransactionId));
+		if (!myextra)
+			return false;
 		*myextra = xid;
 		*extra = myextra;
 	}
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8ae510c623b..0d638e29d00 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -2566,6 +2566,8 @@ check_createrole_self_grant(char **newval, void **extra, GucSource source)
 	list_free(elemlist);
 
 	result = (unsigned *) guc_malloc(LOG, sizeof(unsigned));
+	if (!result)
+		return false;
 	*result = options;
 	*extra = result;
 
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 4ad6e236d69..04cdbbd4c72 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -1087,7 +1087,7 @@ check_application_name(char **newval, void **extra, GucSource source)
 	if (!clean)
 		return false;
 
-	ret = guc_strdup(WARNING, clean);
+	ret = guc_strdup(LOG, clean);
 	if (!ret)
 	{
 		pfree(clean);
@@ -1125,7 +1125,7 @@ check_cluster_name(char **newval, void **extra, GucSource source)
 	if (!clean)
 		return false;
 
-	ret = guc_strdup(WARNING, clean);
+	ret = guc_strdup(LOG, clean);
 	if (!ret)
 	{
 		pfree(clean);
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 719e531eb90..646ba2a78fa 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2730,6 +2730,8 @@ check_synchronized_standby_slots(char **newval, void **extra, GucSource source)
 
 	/* GUC extra value must be guc_malloc'd, not palloc'd */
 	config = (SyncStandbySlotsConfigData *) guc_malloc(LOG, size);
+	if (!config)
+		return false;
 
 	/* Transform the data into SyncStandbySlotsConfigData */
 	config->nslotnames = list_length(elemlist);
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 62f1185859f..181d43e8b17 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -4040,7 +4040,13 @@ check_debug_io_direct(char **newval, void **extra, GucSource source)
 		return result;
 
 	/* Save the flags in *extra, for use by assign_debug_io_direct */
-	*extra = guc_malloc(ERROR, sizeof(int));
+	*extra = guc_malloc(LOG, sizeof(int));
+	if (!*extra)
+	{
+		pfree(rawstring);
+		list_free(elemlist);
+		return false;
+	}
 	*((int *) *extra) = flags;
 
 	return result;
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index 27c0b3c2b04..a07c59ece01 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -1078,7 +1078,9 @@ check_log_connections(char **newval, void **extra, GucSource source)
 	 * We succeeded, so allocate `extra` and save the flags there for use by
 	 * assign_log_connections().
 	 */
-	*extra = guc_malloc(ERROR, sizeof(int));
+	*extra = guc_malloc(LOG, sizeof(int));
+	if (!*extra)
+		return false;
 	*((int *) *extra) = flags;
 
 	return true;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 55ab2da299b..0950a7bd1a2 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3617,7 +3617,9 @@ check_restrict_nonsystem_relation_kind(char **newval, void **extra, GucSource so
 	list_free(elemlist);
 
 	/* Save the flags in *extra, for use by the assign function */
-	*extra = guc_malloc(ERROR, sizeof(int));
+	*extra = guc_malloc(LOG, sizeof(int));
+	if (!*extra)
+		return false;
 	*((int *) *extra) = flags;
 
 	return true;
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 860bbd40d42..b891dab3bf6 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2198,7 +2198,9 @@ check_backtrace_functions(char **newval, void **extra, GucSource source)
 	 * whitespace chars to save some memory, but it doesn't seem worth the
 	 * trouble.
 	 */
-	someval = guc_malloc(ERROR, newvallen + 1 + 1);
+	someval = guc_malloc(LOG, newvallen + 1 + 1);
+	if (!someval)
+		return false;
 	for (i = 0, j = 0; i < newvallen; i++)
 	{
 		if ((*newval)[i] == ',')
@@ -2283,7 +2285,9 @@ check_log_destination(char **newval, void **extra, GucSource source)
 	pfree(rawstring);
 	list_free(elemlist);
 
-	myextra = (int *) guc_malloc(ERROR, sizeof(int));
+	myextra = (int *) guc_malloc(LOG, sizeof(int));
+	if (!myextra)
+		return false;
 	*myextra = newlogdest;
 	*extra = myextra;
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 12192445218..68c55d84491 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4909,10 +4909,10 @@ init_custom_variable(const char *name,
 		 strcmp(name, "pljava.vmoptions") == 0))
 		context = PGC_SUSET;
 
-	gen = (struct config_generic *) guc_malloc(ERROR, sz);
+	gen = (struct config_generic *) guc_malloc(FATAL, sz);
 	memset(gen, 0, sz);
 
-	gen->name = guc_strdup(ERROR, name);
+	gen->name = guc_strdup(FATAL, name);
 	gen->context = context;
 	gen->group = CUSTOM_OPTIONS;
 	gen->short_desc = short_desc;
-- 
2.39.3 (Apple Git-146)