From 9940d2a5a5192fda2633a32055a258ea6a4465cb Mon Sep 17 00:00:00 2001
From: Mats Kindahl <mats@timescale.com>
Date: Thu, 8 Dec 2022 10:00:23 +0100
Subject: Check return value of strdup when starting backend

Inside `BackendInitialize`, if `strdup` returns NULL because it is out
of memory, this will later trigger a server crash because of
dereferencing a null pointer.

This commit fixes this by checking the return values of memory
allocation functions and abort with an error if the memory allocation
fails.

Since the postmaster should not exit when if a memory allocation
failure occurs when trying to start a backend, it will capture any
errors occuring when starting a backend and just continue running.

Fatal errors when starting the backend will still abort the postmaster.
---
 src/backend/postmaster/postmaster.c | 89 ++++++++++++++++-------------
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index f459dab360..baae6fa0d5 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -148,6 +148,19 @@
 #define BACKEND_TYPE_BGWORKER	0x0008	/* bgworker process */
 #define BACKEND_TYPE_ALL		0x000F	/* OR of all the above */
 
+/*
+ * Convenience macro to call a function that can allocate memory, check the
+ * return value, and abort with an out of memory error if it is NULL.
+ */
+#define CHECKED_CALL(STMT)       \
+  do {                                            \
+    if ((STMT) == NULL)                           \
+      ereport(ERROR,                              \
+              (errcode(ERRCODE_OUT_OF_MEMORY),    \
+               errmsg("out of memory"),           \
+               errdetail("Statement is " #STMT))); \
+  } while (0)
+
 /*
  * List of active backends (or child processes anyway; we don't actually
  * know whether a given child has become a backend or is still in the
@@ -615,8 +628,8 @@ PostmasterMain(int argc, char *argv[])
 	 *
 	 * 1. We tell sigaction() to block all signals for the duration of the
 	 * signal handler.  This is faster than our old approach of
-	 * blocking/unblocking explicitly in the signal handler, and it should also
-	 * prevent excessive stack consumption if signals arrive quickly.
+	 * blocking/unblocking explicitly in the signal handler, and it should
+	 * also prevent excessive stack consumption if signals arrive quickly.
 	 *
 	 * 2. We do not set the SA_RESTART flag.  This is because signals will be
 	 * blocked at all times except when ServerLoop is waiting for something to
@@ -704,7 +717,7 @@ PostmasterMain(int argc, char *argv[])
 				break;
 
 			case 'C':
-				output_config_variable = strdup(optarg);
+				CHECKED_CALL(output_config_variable = strdup(optarg));
 				break;
 
 			case 'c':
@@ -735,7 +748,7 @@ PostmasterMain(int argc, char *argv[])
 				}
 
 			case 'D':
-				userDoption = strdup(optarg);
+				CHECKED_CALL(userDoption = strdup(optarg));
 				break;
 
 			case 'd':
@@ -1783,20 +1796,36 @@ ServerLoop(void)
 					break;
 				if (FD_ISSET(ListenSocket[i], &rmask))
 				{
-					Port	   *port;
+					/*
+					 * If any errors occur when trying to spawn the backend,
+					 * we will not abort the postmaster and rather report the
+					 * error and continue running. Note that fatal errors will
+					 * not be caught, so do not throw them unless you want to
+					 * shut down the postmaster.
+					 */
+
+					Port	   *port = NULL;
 
-					port = ConnCreate(ListenSocket[i]);
-					if (port)
+					PG_TRY();
 					{
+						port = ConnCreate(ListenSocket[i]);
 						BackendStartup(port);
-
+					}
+					PG_FINALLY();
+					{
 						/*
 						 * We no longer need the open socket or port structure
-						 * in this process
+						 * in this process, and definitely not if we failed
+						 * spawning a backend.
 						 */
-						StreamClose(port->sock);
-						ConnFree(port);
+						if (port)
+						{
+							if (port->sock != PGINVALID_SOCKET)
+								StreamClose(port->sock);
+							ConnFree(port);
+						}
 					}
+					PG_END_TRY();
 				}
 			}
 		}
@@ -2561,21 +2590,11 @@ ConnCreate(int serverFd)
 {
 	Port	   *port;
 
-	if (!(port = (Port *) calloc(1, sizeof(Port))))
-	{
-		ereport(LOG,
-				(errcode(ERRCODE_OUT_OF_MEMORY),
-				 errmsg("out of memory")));
-		ExitPostmaster(1);
-	}
+	CHECKED_CALL(port = calloc(1, sizeof(Port)));
 
+	/* Caller will deal with freeing anything that needs to be free'd */
 	if (StreamConnection(serverFd, port) != STATUS_OK)
-	{
-		if (port->sock != PGINVALID_SOCKET)
-			StreamClose(port->sock);
-		ConnFree(port);
 		return NULL;
-	}
 
 	return port;
 }
@@ -4054,14 +4073,7 @@ BackendStartup(Port *port)
 	 * Create backend data structure.  Better before the fork() so we can
 	 * handle failure cleanly.
 	 */
-	bn = (Backend *) malloc(sizeof(Backend));
-	if (!bn)
-	{
-		ereport(LOG,
-				(errcode(ERRCODE_OUT_OF_MEMORY),
-				 errmsg("out of memory")));
-		return STATUS_ERROR;
-	}
+	CHECKED_CALL(bn = malloc(sizeof(Backend)));
 
 	/*
 	 * Compute the cancel key that will be assigned to this backend. The
@@ -4275,8 +4287,8 @@ BackendInitialize(Port *port)
 	 * Save remote_host and remote_port in port structure (after this, they
 	 * will appear in log_line_prefix data for log messages).
 	 */
-	port->remote_host = strdup(remote_host);
-	port->remote_port = strdup(remote_port);
+	CHECKED_CALL(port->remote_host = strdup(remote_host));
+	CHECKED_CALL(port->remote_port = strdup(remote_port));
 
 	/* And now we can issue the Log_connections message, if wanted */
 	if (Log_connections)
@@ -4307,7 +4319,10 @@ BackendInitialize(Port *port)
 		ret == 0 &&
 		strspn(remote_host, "0123456789.") < strlen(remote_host) &&
 		strspn(remote_host, "0123456789ABCDEFabcdef:") < strlen(remote_host))
-		port->remote_hostname = strdup(remote_host);
+	{
+		CHECKED_CALL(port->remote_hostname = strdup(remote_host));
+	}
+
 
 	/*
 	 * Ready to begin client interaction.  We will give up and _exit(1) after
@@ -4727,11 +4742,7 @@ retry:
 	 * Struct will be free():d from the callback function that runs on a
 	 * different thread.
 	 */
-	childinfo = malloc(sizeof(win32_deadchild_waitinfo));
-	if (!childinfo)
-		ereport(FATAL,
-				(errcode(ERRCODE_OUT_OF_MEMORY),
-				 errmsg("out of memory")));
+	CHECKED_CALL(childinfo = malloc(sizeof(win32_deadchild_waitinfo)));
 
 	childinfo->procHandle = pi.hProcess;
 	childinfo->procId = pi.dwProcessId;
-- 
2.34.1

