From 54d60bbd075a61c7dd2ef384dc930d726d68ee64 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 3 Oct 2009 18:04:57 +0000 Subject: [PATCH] Fix a couple of issues in recent patch to print updates to postgresql.conf settings: avoid calling superuser() in contexts where it's not defined, don't leak the transient copies of GetConfigOption output, and avoid the whole exercise in postmaster child processes. I found that actually no current caller of GetConfigOption has any use for its internal check of GUC_SUPERUSER_ONLY. But rather than just remove that entirely, it seemed better to add a parameter indicating whether to enforce the check. Per report from Simon and subsequent testing. --- src/backend/utils/misc/guc-file.l | 19 ++++++++++++------- src/backend/utils/misc/guc.c | 12 +++++++++--- src/include/utils/guc.h | 4 ++-- src/timezone/pgtz.c | 6 +++--- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index a425cd48ac..424caea13f 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -4,7 +4,7 @@ * * Copyright (c) 2000-2009, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.61 2009/09/17 21:15:18 petere Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.62 2009/10/03 18:04:57 tgl Exp $ */ %{ @@ -312,21 +312,26 @@ ProcessConfigFile(GucContext context) /* If we got here all the options checked out okay, so apply them. */ for (item = head; item; item = item->next) { - char *pre_value = NULL; + char *pre_value = NULL; - if (context == PGC_SIGHUP) - pre_value = pstrdup(GetConfigOption(item->name)); + /* In SIGHUP cases in the postmaster, report changes */ + if (context == PGC_SIGHUP && !IsUnderPostmaster) + pre_value = pstrdup(GetConfigOption(item->name, false)); if (set_config_option(item->name, item->value, context, PGC_S_FILE, GUC_ACTION_SET, true)) { - if (pre_value && strcmp(pre_value, GetConfigOption(item->name)) != 0) + set_config_sourcefile(item->name, item->filename, + item->sourceline); + if (pre_value && + strcmp(pre_value, GetConfigOption(item->name, false)) != 0) ereport(elevel, (errmsg("parameter \"%s\" changed to \"%s\"", item->name, item->value))); - set_config_sourcefile(item->name, item->filename, - item->sourceline); } + + if (pre_value) + pfree(pre_value); } /* Remember when we last successfully loaded the config file. */ diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index cb743bb55e..4d5aeee85c 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10,7 +10,7 @@ * Written by Peter Eisentraut . * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.519 2009/09/22 23:43:38 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.520 2009/10/03 18:04:57 tgl Exp $ * *-------------------------------------------------------------------- */ @@ -5197,11 +5197,15 @@ SetConfigOption(const char *name, const char *value, * Fetch the current value of the option `name'. If the option doesn't exist, * throw an ereport and don't return. * + * If restrict_superuser is true, we also enforce that only superusers can + * see GUC_SUPERUSER_ONLY variables. This should only be passed as true + * in user-driven calls. + * * The string is *not* allocated for modification and is really only * valid until the next call to configuration related functions. */ const char * -GetConfigOption(const char *name) +GetConfigOption(const char *name, bool restrict_superuser) { struct config_generic *record; static char buffer[256]; @@ -5211,7 +5215,9 @@ GetConfigOption(const char *name) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("unrecognized configuration parameter \"%s\"", name))); - if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser()) + if (restrict_superuser && + (record->flags & GUC_SUPERUSER_ONLY) && + !superuser()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to examine \"%s\"", name))); diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 435a722aa0..ef93d0f27c 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -7,7 +7,7 @@ * Copyright (c) 2000-2009, PostgreSQL Global Development Group * Written by Peter Eisentraut . * - * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.105 2009/09/22 23:43:41 tgl Exp $ + * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.106 2009/10/03 18:04:57 tgl Exp $ *-------------------------------------------------------------------- */ #ifndef GUC_H @@ -249,7 +249,7 @@ extern void DefineCustomEnumVariable( extern void EmitWarningsOnPlaceholders(const char *className); -extern const char *GetConfigOption(const char *name); +extern const char *GetConfigOption(const char *name, bool restrict_superuser); extern const char *GetConfigOptionResetString(const char *name); extern void ProcessConfigFile(GucContext context); extern void InitializeGUCOptions(void); diff --git a/src/timezone/pgtz.c b/src/timezone/pgtz.c index 82a0243878..ea26bbd3d3 100644 --- a/src/timezone/pgtz.c +++ b/src/timezone/pgtz.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/timezone/pgtz.c,v 1.63 2009/06/11 14:49:15 momjian Exp $ + * $PostgreSQL: pgsql/src/timezone/pgtz.c,v 1.64 2009/10/03 18:04:57 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1366,7 +1366,7 @@ pg_timezone_initialize(void) pg_tz *def_tz = NULL; /* Do we need to try to figure the session timezone? */ - if (pg_strcasecmp(GetConfigOption("timezone"), "UNKNOWN") == 0) + if (pg_strcasecmp(GetConfigOption("timezone", false), "UNKNOWN") == 0) { /* Select setting */ def_tz = select_default_timezone(); @@ -1377,7 +1377,7 @@ pg_timezone_initialize(void) } /* What about the log timezone? */ - if (pg_strcasecmp(GetConfigOption("log_timezone"), "UNKNOWN") == 0) + if (pg_strcasecmp(GetConfigOption("log_timezone", false), "UNKNOWN") == 0) { /* Select setting, but don't duplicate work */ if (!def_tz) -- 2.40.0