aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2021-03-18 22:09:41 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2021-03-18 22:22:47 -0400
commit377b7a83007d277d32ef19f7c7590c8668d504cb (patch)
treef541fd99372b0b0921b72308e13e24cdbabf2883 /src
parentd303849b059c3c315e5a8d4239016f8328f3296c (diff)
downloadpostgresql-377b7a83007d277d32ef19f7c7590c8668d504cb.tar.gz
postgresql-377b7a83007d277d32ef19f7c7590c8668d504cb.zip
Don't leak malloc'd strings when a GUC setting is rejected.
Because guc.c prefers to keep all its string values in malloc'd not palloc'd storage, it has to be more careful than usual to avoid leaks. Error exits out of string GUC hook checks failed to clear the proposed value string, and error exits out of ProcessGUCArray() failed to clear the malloc'd results of ParseLongOption(). Found via valgrind testing. This problem is ancient, so back-patch to all supported branches. Discussion: https://postgr.es/m/3816764.1616104288@sss.pgh.pa.us
Diffstat (limited to 'src')
-rw-r--r--src/backend/utils/misc/guc.c71
1 files changed, 47 insertions, 24 deletions
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5a3ca5b765f..997b4b70ee2 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10773,6 +10773,8 @@ ProcessGUCArray(ArrayType *array,
char *s;
char *name;
char *value;
+ char *namecopy;
+ char *valuecopy;
d = array_ref(array, 1, &i,
-1 /* varlenarray */ ,
@@ -10797,13 +10799,18 @@ ProcessGUCArray(ArrayType *array,
continue;
}
- (void) set_config_option(name, value,
+ /* free malloc'd strings immediately to avoid leak upon error */
+ namecopy = pstrdup(name);
+ free(name);
+ valuecopy = pstrdup(value);
+ free(value);
+
+ (void) set_config_option(namecopy, valuecopy,
context, source,
action, true, 0, false);
- free(name);
- if (value)
- free(value);
+ pfree(namecopy);
+ pfree(valuecopy);
pfree(s);
}
}
@@ -11235,34 +11242,50 @@ static bool
call_string_check_hook(struct config_string *conf, char **newval, void **extra,
GucSource source, int elevel)
{
+ volatile bool result = true;
+
/* Quick success if no hook */
if (!conf->check_hook)
return true;
- /* Reset variables that might be set by hook */
- GUC_check_errcode_value = ERRCODE_INVALID_PARAMETER_VALUE;
- GUC_check_errmsg_string = NULL;
- GUC_check_errdetail_string = NULL;
- GUC_check_errhint_string = NULL;
+ /*
+ * If elevel is ERROR, or if the check_hook itself throws an elog
+ * (undesirable, but not always avoidable), make sure we don't leak the
+ * already-malloc'd newval string.
+ */
+ PG_TRY();
+ {
+ /* Reset variables that might be set by hook */
+ GUC_check_errcode_value = ERRCODE_INVALID_PARAMETER_VALUE;
+ GUC_check_errmsg_string = NULL;
+ GUC_check_errdetail_string = NULL;
+ GUC_check_errhint_string = NULL;
- if (!conf->check_hook(newval, extra, source))
+ if (!conf->check_hook(newval, extra, source))
+ {
+ ereport(elevel,
+ (errcode(GUC_check_errcode_value),
+ GUC_check_errmsg_string ?
+ errmsg_internal("%s", GUC_check_errmsg_string) :
+ errmsg("invalid value for parameter \"%s\": \"%s\"",
+ conf->gen.name, *newval ? *newval : ""),
+ GUC_check_errdetail_string ?
+ errdetail_internal("%s", GUC_check_errdetail_string) : 0,
+ GUC_check_errhint_string ?
+ errhint("%s", GUC_check_errhint_string) : 0));
+ /* Flush any strings created in ErrorContext */
+ FlushErrorState();
+ result = false;
+ }
+ }
+ PG_CATCH();
{
- ereport(elevel,
- (errcode(GUC_check_errcode_value),
- GUC_check_errmsg_string ?
- errmsg_internal("%s", GUC_check_errmsg_string) :
- errmsg("invalid value for parameter \"%s\": \"%s\"",
- conf->gen.name, *newval ? *newval : ""),
- GUC_check_errdetail_string ?
- errdetail_internal("%s", GUC_check_errdetail_string) : 0,
- GUC_check_errhint_string ?
- errhint("%s", GUC_check_errhint_string) : 0));
- /* Flush any strings created in ErrorContext */
- FlushErrorState();
- return false;
+ free(*newval);
+ PG_RE_THROW();
}
+ PG_END_TRY();
- return true;
+ return result;
}
static bool