From 0a47d84cc3536138188984d8f2b46ce5a71a8896 Mon Sep 17 00:00:00 2001 From: Justin Pryzby 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