Skip to content

Commit 058b515

Browse files
danielgustafssondanielgustafsson
committed
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
1 parent 043799f commit 058b515

File tree

10 files changed

+36
-13
lines changed

10 files changed

+36
-13
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4791,7 +4791,9 @@ check_wal_consistency_checking(char **newval, void **extra, GucSource source)
47914791
list_free(elemlist);
47924792

47934793
/* assign new value */
4794-
*extra = guc_malloc(ERROR, (RM_MAX_ID + 1) * sizeof(bool));
4794+
*extra = guc_malloc(LOG, (RM_MAX_ID + 1) * sizeof(bool));
4795+
if (!*extra)
4796+
return false;
47954797
memcpy(*extra, newwalconsistency, (RM_MAX_ID + 1) * sizeof(bool));
47964798
return true;
47974799
}

‎src/backend/access/transam/xlogrecovery.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4833,7 +4833,9 @@ check_recovery_target_lsn(char **newval, void **extra, GucSource source)
48334833
if (have_error)
48344834
return false;
48354835

4836-
myextra = (XLogRecPtr *) guc_malloc(ERROR, sizeof(XLogRecPtr));
4836+
myextra = (XLogRecPtr *) guc_malloc(LOG, sizeof(XLogRecPtr));
4837+
if (!myextra)
4838+
return false;
48374839
*myextra = lsn;
48384840
*extra = myextra;
48394841
}
@@ -4997,7 +4999,9 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source)
49974999
}
49985000
}
49995001

5000-
myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(ERROR, sizeof(RecoveryTargetTimeLineGoal));
5002+
myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(LOG, sizeof(RecoveryTargetTimeLineGoal));
5003+
if (!myextra)
5004+
return false;
50015005
*myextra = rttg;
50025006
*extra = myextra;
50035007

@@ -5033,7 +5037,9 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
50335037
if (errno == EINVAL || errno == ERANGE)
50345038
return false;
50355039

5036-
myextra = (TransactionId *) guc_malloc(ERROR, sizeof(TransactionId));
5040+
myextra = (TransactionId *) guc_malloc(LOG, sizeof(TransactionId));
5041+
if (!myextra)
5042+
return false;
50375043
*myextra = xid;
50385044
*extra = myextra;
50395045
}

‎src/backend/commands/user.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2566,6 +2566,8 @@ check_createrole_self_grant(char **newval, void **extra, GucSource source)
25662566
list_free(elemlist);
25672567

25682568
result = (unsigned *) guc_malloc(LOG, sizeof(unsigned));
2569+
if (!result)
2570+
return false;
25692571
*result = options;
25702572
*extra = result;
25712573

‎src/backend/commands/variable.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,7 +1087,7 @@ check_application_name(char **newval, void **extra, GucSource source)
10871087
if (!clean)
10881088
return false;
10891089

1090-
ret = guc_strdup(WARNING, clean);
1090+
ret = guc_strdup(LOG, clean);
10911091
if (!ret)
10921092
{
10931093
pfree(clean);
@@ -1125,7 +1125,7 @@ check_cluster_name(char **newval, void **extra, GucSource source)
11251125
if (!clean)
11261126
return false;
11271127

1128-
ret = guc_strdup(WARNING, clean);
1128+
ret = guc_strdup(LOG, clean);
11291129
if (!ret)
11301130
{
11311131
pfree(clean);

‎src/backend/replication/slot.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2730,6 +2730,8 @@ check_synchronized_standby_slots(char **newval, void **extra, GucSource source)
27302730

27312731
/* GUC extra value must be guc_malloc'd, not palloc'd */
27322732
config = (SyncStandbySlotsConfigData *) guc_malloc(LOG, size);
2733+
if (!config)
2734+
return false;
27332735

27342736
/* Transform the data into SyncStandbySlotsConfigData */
27352737
config->nslotnames = list_length(elemlist);

‎src/backend/storage/file/fd.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4042,7 +4042,9 @@ check_debug_io_direct(char **newval, void **extra, GucSource source)
40424042
return result;
40434043

40444044
/* Save the flags in *extra, for use by assign_debug_io_direct */
4045-
*extra = guc_malloc(ERROR, sizeof(int));
4045+
*extra = guc_malloc(LOG, sizeof(int));
4046+
if (!*extra)
4047+
return false;
40464048
*((int *) *extra) = flags;
40474049

40484050
return result;

‎src/backend/tcop/backend_startup.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1078,7 +1078,9 @@ check_log_connections(char **newval, void **extra, GucSource source)
10781078
* We succeeded, so allocate `extra` and save the flags there for use by
10791079
* assign_log_connections().
10801080
*/
1081-
*extra = guc_malloc(ERROR, sizeof(int));
1081+
*extra = guc_malloc(LOG, sizeof(int));
1082+
if (!*extra)
1083+
return false;
10821084
*((int *) *extra) = flags;
10831085

10841086
return true;

‎src/backend/tcop/postgres.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3648,7 +3648,9 @@ check_restrict_nonsystem_relation_kind(char **newval, void **extra, GucSource so
36483648
list_free(elemlist);
36493649

36503650
/* Save the flags in *extra, for use by the assign function */
3651-
*extra = guc_malloc(ERROR, sizeof(int));
3651+
*extra = guc_malloc(LOG, sizeof(int));
3652+
if (!*extra)
3653+
return false;
36523654
*((int *) *extra) = flags;
36533655

36543656
return true;

‎src/backend/utils/error/elog.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2198,7 +2198,9 @@ check_backtrace_functions(char **newval, void **extra, GucSource source)
21982198
* whitespace chars to save some memory, but it doesn't seem worth the
21992199
* trouble.
22002200
*/
2201-
someval = guc_malloc(ERROR, newvallen + 1 + 1);
2201+
someval = guc_malloc(LOG, newvallen + 1 + 1);
2202+
if (!someval)
2203+
return false;
22022204
for (i = 0, j = 0; i < newvallen; i++)
22032205
{
22042206
if ((*newval)[i] == ',')
@@ -2283,7 +2285,9 @@ check_log_destination(char **newval, void **extra, GucSource source)
22832285
pfree(rawstring);
22842286
list_free(elemlist);
22852287

2286-
myextra = (int *) guc_malloc(ERROR, sizeof(int));
2288+
myextra = (int *) guc_malloc(LOG, sizeof(int));
2289+
if (!myextra)
2290+
return false;
22872291
*myextra = newlogdest;
22882292
*extra = myextra;
22892293

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4909,10 +4909,11 @@ init_custom_variable(const char *name,
49094909
strcmp(name, "pljava.vmoptions") == 0))
49104910
context = PGC_SUSET;
49114911

4912-
gen = (struct config_generic *) guc_malloc(ERROR, sz);
4912+
/* As above, an OOM here is FATAL */
4913+
gen = (struct config_generic *) guc_malloc(FATAL, sz);
49134914
memset(gen, 0, sz);
49144915

4915-
gen->name = guc_strdup(ERROR, name);
4916+
gen->name = guc_strdup(FATAL, name);
49164917
gen->context = context;
49174918
gen->group = CUSTOM_OPTIONS;
49184919
gen->short_desc = short_desc;

0 commit comments

Comments
 (0)