Fix guc_malloc calls for consistency and OOM checks

check_createrole_self_grant and check_synchronized_standby_slots
were allocating memory on a LOG elevel without checking if the
allocation succeeded or not, which would have led to a segfault
on allocation failure.

On top of that, a number of callsites were using the ERROR level,
relying on erroring out rather than returning false to allow the
GUC machinery handle it gracefully.  Other callsites used WARNING
instead of LOG.  While neither being not wrong, this changes all
check_ functions do it consistently with LOG.

init_custom_variable gets a promoted elevel to FATAL to keep
the guc_malloc error handling in line with the rest of the
error handling in that function which already call FATAL.  If
we encounter an OOM in this callsite there is no graceful
handling to be had, better to error out hard.

Backpatch the fix to check_createrole_self_grant down to v16
and the fix to check_synchronized_standby_slots down to v17
where they were introduced.

Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Nikita <pm91.arapov@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Bug: #18845
Discussion: https://postgr.es/m/18845-582c6e10247377ec@postgresql.org
Backpatch-through: 16

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 4b6c694a3f71941b190293d4ef8f346b8fdd3229..fc30a52d496aa266a82f556709f01ec044f05c80 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4791,7 +4791,9 @@ check_wal_consistency_checking(char **newval, void **extra, GucSource source)
    list_free(elemlist);
 
    /* assign new value */
-   *extra = guc_malloc(ERROR, (RM_MAX_ID + 1) * sizeof(bool));
+   *extra = guc_malloc(LOG, (RM_MAX_ID + 1) * sizeof(bool));
+   if (!*extra)
+       return false;
    memcpy(*extra, newwalconsistency, (RM_MAX_ID + 1) * sizeof(bool));
    return true;
 }

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 2c19013c98b27b418f6257e9ca862bf9856a02d9..0aa3ab59085ad4ed8563128e61de6683a1f7cbfc 100644 (file)
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -4833,7 +4833,9 @@ check_recovery_target_lsn(char **newval, void **extra, GucSource source)
        if (have_error)
            return false;
 
-       myextra = (XLogRecPtr *) guc_malloc(ERROR, sizeof(XLogRecPtr));
+       myextra = (XLogRecPtr *) guc_malloc(LOG, sizeof(XLogRecPtr));
+       if (!myextra)
+           return false;
        *myextra = lsn;
        *extra = myextra;
    }
@@ -4997,7 +4999,9 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source)
        }
    }
 
-   myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(ERROR, sizeof(RecoveryTargetTimeLineGoal));
+   myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(LOG, sizeof(RecoveryTargetTimeLineGoal));
+   if (!myextra)
+       return false;
    *myextra = rttg;
    *extra = myextra;
 
@@ -5033,7 +5037,9 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
        if (errno == EINVAL || errno == ERANGE)
            return false;
 
-       myextra = (TransactionId *) guc_malloc(ERROR, sizeof(TransactionId));
+       myextra = (TransactionId *) guc_malloc(LOG, sizeof(TransactionId));
+       if (!myextra)
+           return false;
        *myextra = xid;
        *extra = myextra;
    }

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8ae510c623b9d2c8387cb0060ba74717c5dc1933..0d638e29d0066415abab9a5a898beff3a17c3a8e 100644 (file)
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -2566,6 +2566,8 @@ check_createrole_self_grant(char **newval, void **extra, GucSource source)
    list_free(elemlist);
 
    result = (unsigned *) guc_malloc(LOG, sizeof(unsigned));
+   if (!result)
+       return false;
    *result = options;
    *extra = result;
 

diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index f550a3c0c63b88bf2e15995e3191346a64faf983..84f044a19599cf7e372b9550bf7dd8e4e7e2f616 100644 (file)
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -1087,7 +1087,7 @@ check_application_name(char **newval, void **extra, GucSource source)
    if (!clean)
        return false;
 
-   ret = guc_strdup(WARNING, clean);
+   ret = guc_strdup(LOG, clean);
    if (!ret)
    {
        pfree(clean);
@@ -1125,7 +1125,7 @@ check_cluster_name(char **newval, void **extra, GucSource source)
    if (!clean)
        return false;
 
-   ret = guc_strdup(WARNING, clean);
+   ret = guc_strdup(LOG, clean);
    if (!ret)
    {
        pfree(clean);

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 719e531eb907cb3e0e8d66bf0957846d1c99b00a..646ba2a78fa9cab6dc53af7ac75cecd15d481dcb 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2730,6 +2730,8 @@ check_synchronized_standby_slots(char **newval, void **extra, GucSource source)
 
    /* GUC extra value must be guc_malloc'd, not palloc'd */
    config = (SyncStandbySlotsConfigData *) guc_malloc(LOG, size);
+   if (!config)
+       return false;
 
    /* Transform the data into SyncStandbySlotsConfigData */
    config->nslotnames = list_length(elemlist);

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 0c3a2a756e7b2abfc5b22f2faf13e95ec462c714..84873f4148874f02a0c09c335e1e65f05807bce1 100644 (file)
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -4042,7 +4042,9 @@ check_debug_io_direct(char **newval, void **extra, GucSource source)
        return result;
 
    /* Save the flags in *extra, for use by assign_debug_io_direct */
-   *extra = guc_malloc(ERROR, sizeof(int));
+   *extra = guc_malloc(LOG, sizeof(int));
+   if (!*extra)
+       return false;
    *((int *) *extra) = flags;
 
    return result;

diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index 27c0b3c2b045bd97c9fc6e8037917175bbf540d5..a07c59ece013e0f7b548cc6a50dd27a73b2199fc 100644 (file)
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -1078,7 +1078,9 @@ check_log_connections(char **newval, void **extra, GucSource source)
     * We succeeded, so allocate `extra` and save the flags there for use by
     * assign_log_connections().
     */
-   *extra = guc_malloc(ERROR, sizeof(int));
+   *extra = guc_malloc(LOG, sizeof(int));
+   if (!*extra)
+       return false;
    *((int *) *extra) = flags;
 
    return true;

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 4d2edb10658bea9b572196f25535360adc0ac15b..aec65007bb6a3f6c21d584247954ec1e7e456757 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3648,7 +3648,9 @@ check_restrict_nonsystem_relation_kind(char **newval, void **extra, GucSource so
    list_free(elemlist);
 
    /* Save the flags in *extra, for use by the assign function */
-   *extra = guc_malloc(ERROR, sizeof(int));
+   *extra = guc_malloc(LOG, sizeof(int));
+   if (!*extra)
+       return false;
    *((int *) *extra) = flags;
 
    return true;

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 860bbd40d42b77b2d8a2dddec1e81a1286099f15..b891dab3bf64569bd71bbd3c9bd61fc4e4de2ca7 100644 (file)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2198,7 +2198,9 @@ check_backtrace_functions(char **newval, void **extra, GucSource source)
     * whitespace chars to save some memory, but it doesn't seem worth the
     * trouble.
     */
-   someval = guc_malloc(ERROR, newvallen + 1 + 1);
+   someval = guc_malloc(LOG, newvallen + 1 + 1);
+   if (!someval)
+       return false;
    for (i = 0, j = 0; i < newvallen; i++)
    {
        if ((*newval)[i] == ',')
@@ -2283,7 +2285,9 @@ check_log_destination(char **newval, void **extra, GucSource source)
    pfree(rawstring);
    list_free(elemlist);
 
-   myextra = (int *) guc_malloc(ERROR, sizeof(int));
+   myextra = (int *) guc_malloc(LOG, sizeof(int));
+   if (!myextra)
+       return false;
    *myextra = newlogdest;
    *extra = myextra;
 

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 121924452183ec4740f81a2488ab26019fd19397..667df448732f2e70dbee73ef0231d5de321de149 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4909,10 +4909,11 @@ init_custom_variable(const char *name,
         strcmp(name, "pljava.vmoptions") == 0))
        context = PGC_SUSET;
 
-   gen = (struct config_generic *) guc_malloc(ERROR, sz);
+   /* As above, an OOM here is FATAL */
+   gen = (struct config_generic *) guc_malloc(FATAL, sz);
    memset(gen, 0, sz);
 
-   gen->name = guc_strdup(ERROR, name);
+   gen->name = guc_strdup(FATAL, name);
    gen->context = context;
    gen->group = CUSTOM_OPTIONS;
    gen->short_desc = short_desc;