Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 642b0b6

Browse filesBrowse files
committed
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
1 parent eba9395 commit 642b0b6
Copy full SHA for 642b0b6

File tree

Expand file treeCollapse file tree

1 file changed

+47
-24
lines changed
Filter options
  • src/backend/utils/misc
Expand file treeCollapse file tree

1 file changed

+47
-24
lines changed

‎src/backend/utils/misc/guc.c

Copy file name to clipboardExpand all lines: src/backend/utils/misc/guc.c
+47-24Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10591,6 +10591,8 @@ ProcessGUCArray(ArrayType *array,
1059110591
char *s;
1059210592
char *name;
1059310593
char *value;
10594+
char *namecopy;
10595+
char *valuecopy;
1059410596

1059510597
d = array_ref(array, 1, &i,
1059610598
-1 /* varlenarray */ ,
@@ -10615,13 +10617,18 @@ ProcessGUCArray(ArrayType *array,
1061510617
continue;
1061610618
}
1061710619

10618-
(void) set_config_option(name, value,
10620+
/* free malloc'd strings immediately to avoid leak upon error */
10621+
namecopy = pstrdup(name);
10622+
free(name);
10623+
valuecopy = pstrdup(value);
10624+
free(value);
10625+
10626+
(void) set_config_option(namecopy, valuecopy,
1061910627
context, source,
1062010628
action, true, 0, false);
1062110629

10622-
free(name);
10623-
if (value)
10624-
free(value);
10630+
pfree(namecopy);
10631+
pfree(valuecopy);
1062510632
pfree(s);
1062610633
}
1062710634
}
@@ -11053,34 +11060,50 @@ static bool
1105311060
call_string_check_hook(struct config_string *conf, char **newval, void **extra,
1105411061
GucSource source, int elevel)
1105511062
{
11063+
volatile bool result = true;
11064+
1105611065
/* Quick success if no hook */
1105711066
if (!conf->check_hook)
1105811067
return true;
1105911068

11060-
/* Reset variables that might be set by hook */
11061-
GUC_check_errcode_value = ERRCODE_INVALID_PARAMETER_VALUE;
11062-
GUC_check_errmsg_string = NULL;
11063-
GUC_check_errdetail_string = NULL;
11064-
GUC_check_errhint_string = NULL;
11069+
/*
11070+
* If elevel is ERROR, or if the check_hook itself throws an elog
11071+
* (undesirable, but not always avoidable), make sure we don't leak the
11072+
* already-malloc'd newval string.
11073+
*/
11074+
PG_TRY();
11075+
{
11076+
/* Reset variables that might be set by hook */
11077+
GUC_check_errcode_value = ERRCODE_INVALID_PARAMETER_VALUE;
11078+
GUC_check_errmsg_string = NULL;
11079+
GUC_check_errdetail_string = NULL;
11080+
GUC_check_errhint_string = NULL;
1106511081

11066-
if (!conf->check_hook(newval, extra, source))
11082+
if (!conf->check_hook(newval, extra, source))
11083+
{
11084+
ereport(elevel,
11085+
(errcode(GUC_check_errcode_value),
11086+
GUC_check_errmsg_string ?
11087+
errmsg_internal("%s", GUC_check_errmsg_string) :
11088+
errmsg("invalid value for parameter \"%s\": \"%s\"",
11089+
conf->gen.name, *newval ? *newval : ""),
11090+
GUC_check_errdetail_string ?
11091+
errdetail_internal("%s", GUC_check_errdetail_string) : 0,
11092+
GUC_check_errhint_string ?
11093+
errhint("%s", GUC_check_errhint_string) : 0));
11094+
/* Flush any strings created in ErrorContext */
11095+
FlushErrorState();
11096+
result = false;
11097+
}
11098+
}
11099+
PG_CATCH();
1106711100
{
11068-
ereport(elevel,
11069-
(errcode(GUC_check_errcode_value),
11070-
GUC_check_errmsg_string ?
11071-
errmsg_internal("%s", GUC_check_errmsg_string) :
11072-
errmsg("invalid value for parameter \"%s\": \"%s\"",
11073-
conf->gen.name, *newval ? *newval : ""),
11074-
GUC_check_errdetail_string ?
11075-
errdetail_internal("%s", GUC_check_errdetail_string) : 0,
11076-
GUC_check_errhint_string ?
11077-
errhint("%s", GUC_check_errhint_string) : 0));
11078-
/* Flush any strings created in ErrorContext */
11079-
FlushErrorState();
11080-
return false;
11101+
free(*newval);
11102+
PG_RE_THROW();
1108111103
}
11104+
PG_END_TRY();
1108211105

11083-
return true;
11106+
return result;
1108411107
}
1108511108

1108611109
static bool

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.