From 0a47d84cc3536138188984d8f2b46ce5a71a8896 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 13 Dec 2021 08:42:38 -0600
Subject: [PATCH 1/3] warn if shared_preload_libraries refers to an nonextant
 library

---
 src/backend/utils/fmgr/dfmgr.c                | 100 ++++++++++--------
 src/backend/utils/misc/guc.c                  |  81 +++++++++++++-
 src/include/fmgr.h                            |   1 +
 .../unsafe_tests/expected/rolenames.out       |   1 +
 src/test/regress/expected/rules.out           |   6 ++
 src/test/regress/sql/rules.sql                |   1 +
 6 files changed, 145 insertions(+), 45 deletions(-)

diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index cc1a5200705..474de52d349 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -173,6 +173,62 @@ lookup_external_function(void *filehandle, const char *funcname)
 	return dlsym(filehandle, funcname);
 }
 
+/*
+ * Attempt to load the library with the given filename.
+ * If it fails, issue an error as specified and return NULL.
+ */
+void *
+try_open_library(const char *libname, int errlevel)
+{
+	void *dlh;
+	PGModuleMagicFunction magic_func;
+	const Pg_magic_struct *magic_data_ptr;
+
+	dlh = dlopen(libname, RTLD_NOW | RTLD_GLOBAL);
+	if (dlh == NULL)
+	{
+		char *load_error = dlerror();
+		/* errcode_for_file_access might not be appropriate here? */
+		ereport(errlevel,
+				(errcode_for_file_access(),
+				 errmsg("could not load library: %s",
+						load_error)));
+		return NULL;
+	}
+
+	/* Check the magic function to determine compatibility */
+	magic_func = (PGModuleMagicFunction) dlsym(dlh,
+			PG_MAGIC_FUNCTION_NAME_STRING);
+	if (magic_func == NULL)
+	{
+		/* try to close library */
+		dlclose(dlh);
+		/* complain */
+		ereport(errlevel,
+				(errmsg("incompatible library \"%s\": missing magic block",
+						libname),
+				 errhint("Extension libraries are required to use the PG_MODULE_MAGIC macro.")));
+		return NULL;
+	}
+
+	magic_data_ptr = (*magic_func) ();
+
+	if (magic_data_ptr->len != magic_data.len ||
+		memcmp(magic_data_ptr, &magic_data, magic_data.len) != 0)
+	{
+		/* copy data block before unlinking library */
+		Pg_magic_struct module_magic_data = *magic_data_ptr;
+
+		/* try to close library */
+		dlclose(dlh);
+
+		/* issue suitable complaint */
+		incompatible_module_error(libname, &module_magic_data);
+		return NULL;
+	}
+
+	return dlh;
+}
 
 /*
  * Load the specified dynamic-link library file, unless it already is
@@ -184,8 +240,6 @@ static void *
 internal_load_library(const char *libname)
 {
 	DynamicFileList *file_scanner;
-	PGModuleMagicFunction magic_func;
-	char	   *load_error;
 	struct stat stat_buf;
 	PG_init_t	PG_init;
 
@@ -236,49 +290,11 @@ internal_load_library(const char *libname)
 #endif
 		file_scanner->next = NULL;
 
-		file_scanner->handle = dlopen(file_scanner->filename, RTLD_NOW | RTLD_GLOBAL);
+		file_scanner->handle = try_open_library(libname, ERROR);
 		if (file_scanner->handle == NULL)
 		{
-			load_error = dlerror();
+			/* Not needed since we would've issued an ERROR ? */
 			free((char *) file_scanner);
-			/* errcode_for_file_access might not be appropriate here? */
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not load library \"%s\": %s",
-							libname, load_error)));
-		}
-
-		/* Check the magic function to determine compatibility */
-		magic_func = (PGModuleMagicFunction)
-			dlsym(file_scanner->handle, PG_MAGIC_FUNCTION_NAME_STRING);
-		if (magic_func)
-		{
-			const Pg_magic_struct *magic_data_ptr = (*magic_func) ();
-
-			if (magic_data_ptr->len != magic_data.len ||
-				memcmp(magic_data_ptr, &magic_data, magic_data.len) != 0)
-			{
-				/* copy data block before unlinking library */
-				Pg_magic_struct module_magic_data = *magic_data_ptr;
-
-				/* try to close library */
-				dlclose(file_scanner->handle);
-				free((char *) file_scanner);
-
-				/* issue suitable complaint */
-				incompatible_module_error(libname, &module_magic_data);
-			}
-		}
-		else
-		{
-			/* try to close library */
-			dlclose(file_scanner->handle);
-			free((char *) file_scanner);
-			/* complain */
-			ereport(ERROR,
-					(errmsg("incompatible library \"%s\": missing magic block",
-							libname),
-					 errhint("Extension libraries are required to use the PG_MODULE_MAGIC macro.")));
 		}
 
 		/*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index bff949a40bc..17cfda9d200 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -169,6 +169,12 @@ static bool call_enum_check_hook(struct config_enum *conf, int *newval,
 								 void **extra, GucSource source, int elevel);
 
 static bool check_log_destination(char **newval, void **extra, GucSource source);
+static bool check_local_preload_libraries(char **newval, void **extra,
+		GucSource source);
+static bool check_session_preload_libraries(char **newval, void **extra,
+		GucSource source);
+static bool check_shared_preload_libraries(char **newval, void **extra,
+		GucSource source);
 static void assign_log_destination(const char *newval, void *extra);
 
 static bool check_wal_consistency_checking(char **newval, void **extra,
@@ -4178,7 +4184,7 @@ static struct config_string ConfigureNamesString[] =
 		},
 		&session_preload_libraries_string,
 		"",
-		NULL, NULL, NULL
+		check_session_preload_libraries, NULL, NULL
 	},
 
 	{
@@ -4189,7 +4195,7 @@ static struct config_string ConfigureNamesString[] =
 		},
 		&shared_preload_libraries_string,
 		"",
-		NULL, NULL, NULL
+		check_shared_preload_libraries, NULL, NULL
 	},
 
 	{
@@ -4200,7 +4206,7 @@ static struct config_string ConfigureNamesString[] =
 		},
 		&local_preload_libraries_string,
 		"",
-		NULL, NULL, NULL
+		check_local_preload_libraries, NULL, NULL
 	},
 
 	{
@@ -12133,6 +12139,75 @@ check_max_worker_processes(int *newval, void **extra, GucSource source)
 	return true;
 }
 
+/*
+ * See also load_libraries() and internal_load_library().
+ */
+static bool
+check_preload_libraries(char **newval, void **extra, GucSource source,
+	bool restricted)
+{
+	char		*rawstring;
+	List		*elemlist;
+	ListCell	*l;
+
+	/* nothing to do if empty */
+	if (newval == NULL || *newval[0] == '\0')
+		return true;
+
+	/* Need a modifiable copy of string */
+	rawstring = pstrdup(*newval);
+
+	/* Parse string into list of filename paths */
+	if (!SplitDirectoriesString(rawstring, ',', &elemlist))
+	{
+		/* Should not happen ? */
+		return false;
+	}
+
+	foreach(l, elemlist)
+	{
+		/* Note that filename was already canonicalized */
+		char       *filename = (char *) lfirst(l);
+		char       *expanded = NULL;
+		void *dlh;
+
+		/* If restricting, insert $libdir/plugins if not mentioned already */
+		if (restricted && first_dir_separator(filename) == NULL)
+		{
+			expanded = psprintf("$libdir/plugins/%s", filename);
+			filename = expanded;
+		}
+
+		// stat() or access() ?
+
+		dlh = try_open_library(filename, WARNING);
+		if (dlh && false)
+			; /* Do nothing? dlclose(dlh); */
+	}
+
+	list_free_deep(elemlist);
+	pfree(rawstring);
+	return true;
+}
+
+static bool
+check_shared_preload_libraries(char **newval, void **extra, GucSource source)
+{
+	return check_preload_libraries(newval, extra, source, true);
+}
+
+static bool
+check_local_preload_libraries(char **newval, void **extra, GucSource source)
+{
+	return check_preload_libraries(newval, extra, source, false);
+}
+
+static bool
+check_session_preload_libraries(char **newval, void **extra, GucSource source)
+{
+	return check_preload_libraries(newval, extra, source, true);
+}
+
 static bool
 check_effective_io_concurrency(int *newval, void **extra, GucSource source)
 {
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index cec663bdff0..0c27b617487 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -727,6 +727,7 @@ extern void *load_external_function(const char *filename, const char *funcname,
 									bool signalNotFound, void **filehandle);
 extern void *lookup_external_function(void *filehandle, const char *funcname);
 extern void load_file(const char *filename, bool restricted);
+extern void *try_open_library(const char *libname, int errlevel);
 extern void **find_rendezvous_variable(const char *varName);
 extern Size EstimateLibraryStateSpace(void);
 extern void SerializeLibraryState(Size maxsize, char *start_address);
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index eb608fdc2ea..bd45cd26c43 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1066,6 +1066,7 @@ GRANT pg_read_all_settings TO regress_role_haspriv;
 BEGIN;
 -- A GUC using GUC_SUPERUSER_ONLY is useful for negative tests.
 SET LOCAL session_preload_libraries TO 'path-to-preload-libraries';
+WARNING:  could not load library: $libdir/plugins/path-to-preload-libraries: cannot open shared object file: No such file or directory
 SET SESSION AUTHORIZATION regress_role_haspriv;
 -- passes with role member of pg_read_all_settings
 SHOW session_preload_libraries;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index b58b062b10d..d8bf63886c5 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3572,6 +3572,7 @@ drop table hats;
 drop table hat_data;
 -- test for pg_get_functiondef properly regurgitating SET parameters
 -- Note that the function is kept around to stress pg_dump.
+SET check_function_bodies=no;
 CREATE FUNCTION func_with_set_params() RETURNS integer
     AS 'select 1;'
     LANGUAGE SQL
@@ -3581,6 +3582,11 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
     SET datestyle to iso, mdy
     SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
     IMMUTABLE STRICT;
+WARNING:  could not load library: Mixed/Case: cannot open shared object file: No such file or directory
+WARNING:  could not load library: c:/'a"/path: cannot open shared object file: No such file or directory
+WARNING:  incompatible library "": missing magic block
+HINT:  Extension libraries are required to use the PG_MODULE_MAGIC macro.
+WARNING:  could not load library: 0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789: cannot open shared object file: No such file or directory
 SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
                                                                             pg_get_functiondef                                                                            
 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index b732833e63c..1e42d092862 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1198,6 +1198,7 @@ drop table hat_data;
 
 -- test for pg_get_functiondef properly regurgitating SET parameters
 -- Note that the function is kept around to stress pg_dump.
+SET check_function_bodies=no;
 CREATE FUNCTION func_with_set_params() RETURNS integer
     AS 'select 1;'
     LANGUAGE SQL
-- 
2.17.1

