v6-0001-Disallow-external-access-to-MaxBackends.patch

From 615dfbd3352a235aa615098a13e3de33edb60c5a Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Fri, 7 Jan 2022 19:50:57 +0000
Subject: [PATCH v6 1/1] Disallow external access to MaxBackends.

Presently, MaxBackends is externally visible, but it may still be
uninitialized in places where it would be convenient to use (e.g.,
_PG_init()).  This change renames MaxBackends to
MaxBackendsInternal, makes it static to postinit.c to disallow
direct access, and introduces a MaxBackends macro for calling
GetMaxBackends().

Separately, adjust the comments about needing to register
background workers before initializing MaxBackends.  Since
6bc8ef0b, InitializeMaxBackends() has used max_worker_processes
instead of tallying up the number of registered background workers,
so background worker registration is no longer a prerequisite.  The
ordering of this logic is still useful for allowing libraries to
adjust GUCs, so the comments have been updated to mention that use-
case.
---
 src/backend/postmaster/postmaster.c | 16 ++++++-------
 src/backend/utils/init/globals.c    |  4 ----
 src/backend/utils/init/postinit.c   | 47 ++++++++++++++++++++++++++++++-------
 src/include/miscadmin.h             |  5 +++-
 4 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 328ecafa8c..1609e24114 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -525,7 +525,7 @@ typedef struct
 	bool		IsBinaryUpgrade;
 	bool		query_id_enabled;
 	int			max_safe_fds;
-	int			MaxBackends;
+	int			max_backends;
 #ifdef WIN32
 	HANDLE		PostmasterHandle;
 	HANDLE		initial_signal_pipe;
@@ -1014,10 +1014,8 @@ PostmasterMain(int argc, char *argv[])
 	LocalProcessControlFile(false);
 
 	/*
-	 * Register the apply launcher.  Since it registers a background worker,
-	 * it needs to be called before InitializeMaxBackends(), and it's probably
-	 * a good idea to call it before any modules had chance to take the
-	 * background worker slots.
+	 * Register the apply launcher.  It's probably a good idea to call it before
+	 * any modules had chance to take the background worker slots.
 	 */
 	ApplyLauncherRegister();
 
@@ -1038,8 +1036,8 @@ PostmasterMain(int argc, char *argv[])
 #endif
 
 	/*
-	 * Now that loadable modules have had their chance to register background
-	 * workers, calculate MaxBackends.
+	 * Now that loadable modules have had their chance to alter any GUCs,
+	 * calculate MaxBackends.
 	 */
 	InitializeMaxBackends();
 
@@ -6262,7 +6260,7 @@ save_backend_variables(BackendParameters *param, Port *port,
 	param->query_id_enabled = query_id_enabled;
 	param->max_safe_fds = max_safe_fds;
 
-	param->MaxBackends = MaxBackends;
+	param->max_backends = MaxBackends;
 
 #ifdef WIN32
 	param->PostmasterHandle = PostmasterHandle;
@@ -6496,7 +6494,7 @@ restore_backend_variables(BackendParameters *param, Port *port)
 	query_id_enabled = param->query_id_enabled;
 	max_safe_fds = param->max_safe_fds;
 
-	MaxBackends = param->MaxBackends;
+	SetMaxBackends(param->max_backends);
 
 #ifdef WIN32
 	PostmasterHandle = param->PostmasterHandle;
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 381d9e548d..47884bbe85 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -128,15 +128,11 @@ int			max_parallel_maintenance_workers = 2;
 
 /*
  * Primary determinants of sizes of shared-memory structures.
- *
- * MaxBackends is computed by PostmasterMain after modules have had a chance to
- * register background workers.
  */
 int			NBuffers = 1000;
 int			MaxConnections = 90;
 int			max_worker_processes = 8;
 int			max_parallel_workers = 8;
-int			MaxBackends = 0;
 
 int			VacuumCostPageHit = 1;	/* GUC parameters for vacuum */
 int			VacuumCostPageMiss = 2;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 7292e51f7d..aeeb709a67 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -63,6 +63,9 @@
 #include "utils/syscache.h"
 #include "utils/timeout.h"
 
+static bool MaxBackendsInitialized = false;
+static int MaxBackendsInternal = 0;
+
 static HeapTuple GetDatabaseTuple(const char *dbname);
 static HeapTuple GetDatabaseTupleByOid(Oid dboid);
 static void PerformAuthentication(Port *port);
@@ -476,9 +479,8 @@ pg_split_opts(char **argv, int *argcp, const char *optstr)
 /*
  * Initialize MaxBackends value from config options.
  *
- * This must be called after modules have had the chance to register background
- * workers in shared_preload_libraries, and before shared memory size is
- * determined.
+ * This must be called after modules have had the chance to alter GUCs in
+ * shared_preload_libraries, and before shared memory size is determined.
  *
  * Note that in EXEC_BACKEND environment, the value is passed down from
  * postmaster to subprocesses via BackendParameters in SubPostmasterMain; only
@@ -488,15 +490,42 @@ pg_split_opts(char **argv, int *argcp, const char *optstr)
 void
 InitializeMaxBackends(void)
 {
-	Assert(MaxBackends == 0);
-
 	/* the extra unit accounts for the autovacuum launcher */
-	MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
-		max_worker_processes + max_wal_senders;
+	SetMaxBackends(MaxConnections + autovacuum_max_workers + 1 +
+		max_worker_processes + max_wal_senders);
+}
 
-	/* internal error because the values were all checked previously */
-	if (MaxBackends > MAX_BACKENDS)
+/*
+ * Retrieve the value for MaxBackends.
+ *
+ * If the value has not yet been initialized, this function will ERROR.
+ */
+int
+GetMaxBackends(void)
+{
+	if (!MaxBackendsInitialized)
+		elog(ERROR, "MaxBackends not yet initialized");
+
+	return MaxBackendsInternal;
+}
+
+/*
+ * Set the value for MaxBackends.
+ *
+ * If the value is already initialized or the value provided is not valid, this
+ * function will ERROR.
+ */
+void
+SetMaxBackends(int max_backends)
+{
+	if (MaxBackendsInitialized)
+		elog(ERROR, "MaxBackends already initialized");
+
+	if (max_backends > MAX_BACKENDS)
 		elog(ERROR, "too many backends configured");
+
+	MaxBackendsInternal = max_backends;
+	MaxBackendsInitialized = true;
 }
 
 /*
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 90a3016065..4860a9540a 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -172,7 +172,6 @@ extern PGDLLIMPORT char *DataDir;
 extern PGDLLIMPORT int data_directory_mode;
 
 extern PGDLLIMPORT int NBuffers;
-extern PGDLLIMPORT int MaxBackends;
 extern PGDLLIMPORT int MaxConnections;
 extern PGDLLIMPORT int max_worker_processes;
 extern PGDLLIMPORT int max_parallel_workers;
@@ -457,10 +456,14 @@ extern AuxProcType MyAuxProcType;
 /* in utils/init/postinit.c */
 extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
 extern void InitializeMaxBackends(void);
+extern int GetMaxBackends(void);
+extern void SetMaxBackends(int max_backends);
 extern void InitPostgres(const char *in_dbname, Oid dboid, const char *username,
 						 Oid useroid, char *out_dbname, bool override_allow_connections);
 extern void BaseInit(void);
 
+#define MaxBackends		(GetMaxBackends())
+
 /* in utils/init/miscinit.c */
 extern bool IgnoreSystemIndexes;
 extern PGDLLIMPORT bool process_shared_preload_libraries_in_progress;
-- 
2.16.6