From 2834cbbbdd0794346eb6f78e9799c563b18b135c Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Mon, 25 Mar 2024 22:07:09 +0100 Subject: [PATCH 3/5] Don't clobber any environment variables for ps status Clobbering the environment area for ps status is formally undefined behavior and thus makes up a bad default. Here we replace the clobbering of the environment with a different mechanisum to increase the available argv area for ps status: If the given arguments have not been long enough, yet, we exec ourself with an extra argument of spaces. This argument of spaces is then removed again for further processing, but the additional space is used for ps status. This a more portable approach, because it doesn't rely on undefined behavior and can possibly be made the default instead of PS_USE_NONE later. It also allows to remove the workaround for UBSan in main.c and makes /proc//environ work again. --- src/backend/main/main.c | 41 ++--------- src/backend/utils/misc/ps_status.c | 114 +++++++++++++---------------- src/include/utils/ps_status.h | 2 +- 3 files changed, 58 insertions(+), 99 deletions(-) diff --git a/src/backend/main/main.c b/src/backend/main/main.c index bfd0c5ed658..f2e333d74eb 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -80,14 +80,16 @@ main(int argc, char *argv[]) * Remember the physical location of the initially given argv[] array for * possible use by ps display. On some platforms, the argv[] storage must * be overwritten in order to set the process title for ps. In such cases - * save_ps_display_args makes and returns a new copy of the argv[] array. + * save_ps_display_args makes a new copy of the argv[] array. * - * save_ps_display_args may also move the environment strings to make - * extra room. Therefore this should be done as early as possible during - * startup, to avoid entanglements with code that might save a getenv() - * result pointer. + * save_ps_display_args may also exec ourself again with an extra argument + * to increase the available argv area. Therefore this should be done as + * early as possible during startup, to avoid doing more things than + * necessary twice. Once called with an the extra argument, this will be + * removed again by save_ps_display_args, which then returns the new + * number for argc. */ - argv = save_ps_display_args(argc, argv); + argc = save_ps_display_args(argc, argv); /* * Fire up essential subsystems: error and memory management @@ -414,30 +416,3 @@ check_root(const char *progname) } #endif /* WIN32 */ } - -/* - * At least on linux, set_ps_display() breaks /proc/$pid/environ. The - * sanitizer library uses /proc/$pid/environ to implement getenv() as it wants - * to work independent of libc. When just using undefined and alignment - * sanitizers, the sanitizer library is only initialized when the first error - * occurs, by which time we've often already called set_ps_display(), - * preventing the sanitizer libraries from seeing the options. - * - * We can work around that by defining __ubsan_default_options, a weak symbol - * libsanitizer uses to get defaults from the application, and return - * getenv("UBSAN_OPTIONS"). But only if main already was reached, so that we - * don't end up relying on a not-yet-working getenv(). - * - * As this function won't get called when not running a sanitizer, it doesn't - * seem necessary to only compile it conditionally. - */ -const char *__ubsan_default_options(void); -const char * -__ubsan_default_options(void) -{ - /* don't call libc before it's guaranteed to be initialized */ - if (!reached_main) - return ""; - - return getenv("UBSAN_OPTIONS"); -} diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c index d3513b22693..c1161e85507 100644 --- a/src/backend/utils/misc/ps_status.c +++ b/src/backend/utils/misc/ps_status.c @@ -23,8 +23,6 @@ #include "utils/guc.h" #include "utils/ps_status.h" -extern char **environ; - /* GUC variable */ bool update_process_title = DEFAULT_UPDATE_PROCESS_TITLE; @@ -38,7 +36,7 @@ bool update_process_title = DEFAULT_UPDATE_PROCESS_TITLE; * use the function setproctitle(const char *, ...) * (other BSDs) * PS_USE_CLOBBER_ARGV - * write over the argv and environment area + * write over the argv area * (Linux and most SysV-like systems) * PS_USE_WIN32 * push the string out as the name of a Windows event @@ -78,6 +76,7 @@ bool update_process_title = DEFAULT_UPDATE_PROCESS_TITLE; static char ps_buffer[PS_BUFFER_SIZE]; static const size_t ps_buffer_size = PS_BUFFER_SIZE; #else /* PS_USE_CLOBBER_ARGV */ +#define PS_BUFFER_MIN_SIZE 256 static char *ps_buffer; /* will point to argv area */ static size_t ps_buffer_size; /* space determined at run time */ static size_t last_status_len; /* use to minimize length of clobber */ @@ -107,15 +106,13 @@ static char **save_argv; * If needed, we make a copy of the original argv[] array to preserve it * from being clobbered by subsequent ps_display actions. * - * (The original argv[] will not be overwritten by this routine, but may be - * overwritten during init_ps_display. Also, the physical location of the - * environment strings may be moved, so this should be called before any code - * that might try to hang onto a getenv() result.) + * The original argv[] will not be overwritten by this routine, but may be + * overwritten during init_ps_display. * * Note that in case of failure this cannot call elog() as that is not * initialized yet. We rely on write_stderr() instead. */ -char ** +int save_ps_display_args(int argc, char **argv) { save_argc = argc; @@ -125,11 +122,9 @@ save_ps_display_args(int argc, char **argv) /* * If we're going to overwrite the argv area, count the available space. - * Also move the environment to make additional room. */ { char *end_of_area = NULL; - char **new_environ; int i; /* @@ -145,71 +140,60 @@ save_ps_display_args(int argc, char **argv) { ps_buffer = NULL; ps_buffer_size = 0; - return argv; - } - - /* - * check for contiguous environ strings following argv - */ - for (i = 0; environ[i] != NULL; i++) - { - if (end_of_area + 1 == environ[i]) - { - -#if defined(__linux__) && (! defined(__GLIBC__) && ! defined(__UCLIBC__ )) - /* - * The musl runtime linker stores pointers to variable values - * which are defined in the process's environment. Therefore, - * in these cases we cannot overwrite such variable values - * when setting the process title or dynamic linking (dlopen) - * might fail. Here, we truncate the update of the process - * title when either of two important dynamic linking - * environment variables are set. Musl does not define any - * compiler symbols, so we have to do this for any Linux libc - * we don't know is safe. - */ - if (strstr(environ[i], "LD_LIBRARY_PATH=") == environ[i] || - strstr(environ[i], "LD_PRELOAD=") == environ[i]) - { - /* - * We can overwrite the name, but stop at the equals sign. - * Future loops will not find contiguous space, but we - * don't break early because we want to count the total - * number. - */ - end_of_area = strchr(environ[i], '='); - } - else -#endif - { - end_of_area = environ[i] + strlen(environ[i]); - } - } + return argc; } ps_buffer = argv[0]; last_status_len = ps_buffer_size = end_of_area - argv[0]; + } - /* - * move the environment out of the way - */ - new_environ = (char **) malloc((i + 1) * sizeof(char *)); - if (!new_environ) - { - write_stderr("out of memory\n"); - exit(1); - } - for (i = 0; environ[i] != NULL; i++) + /* + * If the argv area wasn't big enough, also exec ourself an additional + * argument to make more room. + * + * Create a new argument which is just a big number of spaces. While this + * could technically be a valid filename and thus a valid argument to one + * of the other options, it should still be safe enough to break the case + * of passing exactly 256 spaces as a path. If someone really wants to do + * that, they could work around this, by just adding another argument + * after that, because we match the very last argument to this pattern. + */ + { + char *extra_arg; + + extra_arg = (char *) malloc(PS_BUFFER_MIN_SIZE); + MemSet(extra_arg, ' ', PS_BUFFER_MIN_SIZE - 1); + extra_arg[PS_BUFFER_MIN_SIZE - 1] = NULL; + + if (ps_buffer_size <= PS_BUFFER_MIN_SIZE) { - new_environ[i] = strdup(environ[i]); - if (!new_environ[i]) + char **padded_argv; + int i; + + padded_argv = (char **) malloc((argc + 2) * sizeof(char *)); + if (!padded_argv) { write_stderr("out of memory\n"); exit(1); } + for (i = 0; i < argc; i++) + padded_argv[i] = argv[i]; + padded_argv[argc] = extra_arg; + padded_argv[argc + 1] = NULL; + + execvp(argv[0], padded_argv); + write_stderr("exec failed: %s\n", strerror(errno)); + exit(1); + } + else if (strcmp(argv[argc - 1], extra_arg) == 0) + { + /* + * To hide the extra_arg spaces on the ps output for the + * postmaster, we need to overwrite them with PS_PADDING. + */ + MemSet(argv[argc - 1], PS_PADDING, PS_BUFFER_MIN_SIZE - 1); + save_argc = --argc; } - new_environ[i] = NULL; - environ = new_environ; } /* @@ -258,7 +242,7 @@ save_ps_display_args(int argc, char **argv) } #endif /* PS_USE_CLOBBER_ARGV */ - return argv; + return argc; } /* diff --git a/src/include/utils/ps_status.h b/src/include/utils/ps_status.h index ff5a2b2b8a2..7cd2cce8f2c 100644 --- a/src/include/utils/ps_status.h +++ b/src/include/utils/ps_status.h @@ -21,7 +21,7 @@ extern PGDLLIMPORT bool update_process_title; -extern char **save_ps_display_args(int argc, char **argv); +extern int save_ps_display_args(int argc, char **argv); extern void init_ps_display(const char *fixed_part); -- 2.44.0