postgres.git / summary / log / commit / refs

commit    a5c77e6b8cd59949c6443d1371e5c420d1f07bc2
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Mon Oct 12 17:31:24 2020 +0000

    Fix memory leak when guc.c decides a setting can't be applied now.
    
    The prohibitValueChange code paths in set_config_option(), which
    are executed whenever we re-read a PGC_POSTMASTER variable from
    postgresql.conf, neglected to free anything before exiting.  Thus
    we'd leak the proposed new value of a PGC_STRING variable, as noted
    by BoChen in bug #16666.  For all variable types, if the check hook
    creates an "extra" chunk, we'd also leak that.
    
    These are malloc not palloc chunks, so there is no mechanism for
    recovering the leaks before process exit.  Fortunately, the values
    are typically not very large, meaning you'd have to go through an
    awful lot of SIGHUP configuration-reload cycles to make the leakage
    amount to anything.  Still, for a long-lived postmaster process it
    could potentially be a problem.
    
    Oversight in commit 2594cf0e8.  Back-patch to all supported branches.
    
    Discussion: https://postgr.es/m/16666-2c41a4eec61b03e1@postgresql.org


 src/backend/utils/misc/guc.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 454e6b1a384..9f47bd0aea9 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6202,6 +6202,10 @@ set_config_option(const char *name, const char *value,
 
 				if (prohibitValueChange)
 				{
+					/* Release newextra, unless it's reset_extra */
+					if (newextra && !extra_field_used(&conf->gen, newextra))
+						free(newextra);
+
 					if (*conf->variable != newval)
 					{
 						record->status |= GUC_PENDING_RESTART;
@@ -6292,6 +6296,10 @@ set_config_option(const char *name, const char *value,
 
 				if (prohibitValueChange)
 				{
+					/* Release newextra, unless it's reset_extra */
+					if (newextra && !extra_field_used(&conf->gen, newextra))
+						free(newextra);
+
 					if (*conf->variable != newval)
 					{
 						record->status |= GUC_PENDING_RESTART;
@@ -6382,6 +6390,10 @@ set_config_option(const char *name, const char *value,
 
 				if (prohibitValueChange)
 				{
+					/* Release newextra, unless it's reset_extra */
+					if (newextra && !extra_field_used(&conf->gen, newextra))
+						free(newextra);
+
 					if (*conf->variable != newval)
 					{
 						record->status |= GUC_PENDING_RESTART;
@@ -6488,9 +6500,21 @@ set_config_option(const char *name, const char *value,
 
 				if (prohibitValueChange)
 				{
+					bool		newval_different;
+
 					/* newval shouldn't be NULL, so we're a bit sloppy here */
-					if (*conf->variable == NULL || newval == NULL ||
-						strcmp(*conf->variable, newval) != 0)
+					newval_different = (*conf->variable == NULL ||
+										newval == NULL ||
+										strcmp(*conf->variable, newval) != 0);
+
+					/* Release newval, unless it's reset_val */
+					if (newval && !string_field_used(conf, newval))
+						free(newval);
+					/* Release newextra, unless it's reset_extra */
+					if (newextra && !extra_field_used(&conf->gen, newextra))
+						free(newextra);
+
+					if (newval_different)
 					{
 						record->status |= GUC_PENDING_RESTART;
 						ereport(elevel,
@@ -6585,6 +6609,10 @@ set_config_option(const char *name, const char *value,
 
 				if (prohibitValueChange)
 				{
+					/* Release newextra, unless it's reset_extra */
+					if (newextra && !extra_field_used(&conf->gen, newextra))
+						free(newextra);
+
 					if (*conf->variable != newval)
 					{
 						record->status |= GUC_PENDING_RESTART;


[parent: 9f3d3fb8702b]