Commit 26d538dc authored by Tom Lane's avatar Tom Lane

Clean up some lack-of-STRICT issues in the core code, too.

A scan for missed proisstrict markings in the core code turned up
these functions:

brin_summarize_new_values
pg_stat_reset_single_table_counters
pg_stat_reset_single_function_counters
pg_create_logical_replication_slot
pg_create_physical_replication_slot
pg_drop_replication_slot

The first three of these take OID, so a null argument will normally look
like a zero to them, resulting in "ERROR: could not open relation with OID
0" for brin_summarize_new_values, and no action for the pg_stat_reset_XXX
functions.  The other three will dump core on a null argument, though this
is mitigated by the fact that they won't do so until after checking that
the caller is superuser or has rolreplication privilege.

In addition, the pg_logical_slot_get/peek[_binary]_changes family was
intentionally marked nonstrict, but failed to make nullness checks on all
the arguments; so again a null-pointer-dereference crash is possible but
only for superusers and rolreplication users.

Add the missing ARGISNULL checks to the latter functions, and mark the
former functions as strict in pg_proc.  Make that change in the back
branches too, even though we can't force initdb there, just so that
installations initdb'd in future won't have the issue.  Since none of these
bugs rise to the level of security issues (and indeed the pg_stat_reset_XXX
functions hardly misbehave at all), it seems sufficient to do this.

In addition, fix some order-of-operations oddities in the slot_get_changes
family, mostly cosmetic, but not the part that moves the function's last
few operations into the PG_TRY block.  As it stood, there was significant
risk for an error to exit without clearing historical information from
the system caches.

The slot_get_changes bugs go back to 9.4 where that code was introduced.
Back-patch appropriate subsets of the pg_proc changes into all active
branches, as well.
parent 1cb63c79
...@@ -946,6 +946,7 @@ CREATE OR REPLACE FUNCTION pg_create_physical_replication_slot( ...@@ -946,6 +946,7 @@ CREATE OR REPLACE FUNCTION pg_create_physical_replication_slot(
OUT slot_name name, OUT xlog_position pg_lsn) OUT slot_name name, OUT xlog_position pg_lsn)
RETURNS RECORD RETURNS RECORD
LANGUAGE INTERNAL LANGUAGE INTERNAL
STRICT VOLATILE
AS 'pg_create_physical_replication_slot'; AS 'pg_create_physical_replication_slot';
CREATE OR REPLACE FUNCTION CREATE OR REPLACE FUNCTION
......
...@@ -276,25 +276,31 @@ logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, ...@@ -276,25 +276,31 @@ logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
static Datum static Datum
pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool binary) pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool binary)
{ {
Name name = PG_GETARG_NAME(0); Name name;
XLogRecPtr upto_lsn; XLogRecPtr upto_lsn;
int32 upto_nchanges; int32 upto_nchanges;
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
MemoryContext per_query_ctx; MemoryContext per_query_ctx;
MemoryContext oldcontext; MemoryContext oldcontext;
XLogRecPtr end_of_wal; XLogRecPtr end_of_wal;
XLogRecPtr startptr; XLogRecPtr startptr;
LogicalDecodingContext *ctx; LogicalDecodingContext *ctx;
ResourceOwner old_resowner = CurrentResourceOwner; ResourceOwner old_resowner = CurrentResourceOwner;
ArrayType *arr; ArrayType *arr;
Size ndim; Size ndim;
List *options = NIL; List *options = NIL;
DecodingOutputState *p; DecodingOutputState *p;
check_permissions();
CheckLogicalDecodingRequirements();
if (PG_ARGISNULL(0))
ereport(ERROR,
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
errmsg("slot name must not be null")));
name = PG_GETARG_NAME(0);
if (PG_ARGISNULL(1)) if (PG_ARGISNULL(1))
upto_lsn = InvalidXLogRecPtr; upto_lsn = InvalidXLogRecPtr;
else else
...@@ -305,6 +311,12 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin ...@@ -305,6 +311,12 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
else else
upto_nchanges = PG_GETARG_INT32(2); upto_nchanges = PG_GETARG_INT32(2);
if (PG_ARGISNULL(3))
ereport(ERROR,
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
errmsg("options array must not be null")));
arr = PG_GETARG_ARRAYTYPE_P(3);
/* check to see if caller supports us returning a tuplestore */ /* check to see if caller supports us returning a tuplestore */
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
ereport(ERROR, ereport(ERROR,
...@@ -324,16 +336,11 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin ...@@ -324,16 +336,11 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
if (get_call_result_type(fcinfo, NULL, &p->tupdesc) != TYPEFUNC_COMPOSITE) if (get_call_result_type(fcinfo, NULL, &p->tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type"); elog(ERROR, "return type must be a row type");
check_permissions();
CheckLogicalDecodingRequirements();
arr = PG_GETARG_ARRAYTYPE_P(3);
ndim = ARR_NDIM(arr);
per_query_ctx = rsinfo->econtext->ecxt_per_query_memory; per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
oldcontext = MemoryContextSwitchTo(per_query_ctx); oldcontext = MemoryContextSwitchTo(per_query_ctx);
/* Deconstruct options array */
ndim = ARR_NDIM(arr);
if (ndim > 1) if (ndim > 1)
{ {
ereport(ERROR, ereport(ERROR,
...@@ -382,7 +389,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin ...@@ -382,7 +389,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
else else
end_of_wal = GetXLogReplayRecPtr(NULL); end_of_wal = GetXLogReplayRecPtr(NULL);
CheckLogicalDecodingRequirements();
ReplicationSlotAcquire(NameStr(*name)); ReplicationSlotAcquire(NameStr(*name));
PG_TRY(); PG_TRY();
...@@ -444,15 +450,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin ...@@ -444,15 +450,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
break; break;
CHECK_FOR_INTERRUPTS(); CHECK_FOR_INTERRUPTS();
} }
}
PG_CATCH();
{
/* clear all timetravel entries */
InvalidateSystemCaches();
PG_RE_THROW();
}
PG_END_TRY();
tuplestore_donestoring(tupstore); tuplestore_donestoring(tupstore);
...@@ -470,6 +467,15 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin ...@@ -470,6 +467,15 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
ReplicationSlotRelease(); ReplicationSlotRelease();
InvalidateSystemCaches(); InvalidateSystemCaches();
}
PG_CATCH();
{
/* clear all timetravel entries */
InvalidateSystemCaches();
PG_RE_THROW();
}
PG_END_TRY();
return (Datum) 0; return (Datum) 0;
} }
...@@ -480,9 +486,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin ...@@ -480,9 +486,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
Datum Datum
pg_logical_slot_get_changes(PG_FUNCTION_ARGS) pg_logical_slot_get_changes(PG_FUNCTION_ARGS)
{ {
Datum ret = pg_logical_slot_get_changes_guts(fcinfo, true, false); return pg_logical_slot_get_changes_guts(fcinfo, true, false);
return ret;
} }
/* /*
...@@ -491,9 +495,7 @@ pg_logical_slot_get_changes(PG_FUNCTION_ARGS) ...@@ -491,9 +495,7 @@ pg_logical_slot_get_changes(PG_FUNCTION_ARGS)
Datum Datum
pg_logical_slot_peek_changes(PG_FUNCTION_ARGS) pg_logical_slot_peek_changes(PG_FUNCTION_ARGS)
{ {
Datum ret = pg_logical_slot_get_changes_guts(fcinfo, false, false); return pg_logical_slot_get_changes_guts(fcinfo, false, false);
return ret;
} }
/* /*
...@@ -502,9 +504,7 @@ pg_logical_slot_peek_changes(PG_FUNCTION_ARGS) ...@@ -502,9 +504,7 @@ pg_logical_slot_peek_changes(PG_FUNCTION_ARGS)
Datum Datum
pg_logical_slot_get_binary_changes(PG_FUNCTION_ARGS) pg_logical_slot_get_binary_changes(PG_FUNCTION_ARGS)
{ {
Datum ret = pg_logical_slot_get_changes_guts(fcinfo, true, true); return pg_logical_slot_get_changes_guts(fcinfo, true, true);
return ret;
} }
/* /*
...@@ -513,7 +513,5 @@ pg_logical_slot_get_binary_changes(PG_FUNCTION_ARGS) ...@@ -513,7 +513,5 @@ pg_logical_slot_get_binary_changes(PG_FUNCTION_ARGS)
Datum Datum
pg_logical_slot_peek_binary_changes(PG_FUNCTION_ARGS) pg_logical_slot_peek_binary_changes(PG_FUNCTION_ARGS)
{ {
Datum ret = pg_logical_slot_get_changes_guts(fcinfo, false, true); return pg_logical_slot_get_changes_guts(fcinfo, false, true);
return ret;
} }
...@@ -53,6 +53,6 @@ ...@@ -53,6 +53,6 @@
*/ */
/* yyyymmddN */ /* yyyymmddN */
#define CATALOG_VERSION_NO 201601071 #define CATALOG_VERSION_NO 201601091
#endif #endif
...@@ -605,7 +605,7 @@ DATA(insert OID = 3800 ( brincostestimate PGNSP PGUID 12 1 0 0 0 f f f f t f v ...@@ -605,7 +605,7 @@ DATA(insert OID = 3800 ( brincostestimate PGNSP PGUID 12 1 0 0 0 f f f f t f v
DESCR("brin(internal)"); DESCR("brin(internal)");
DATA(insert OID = 3801 ( brinoptions PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 17 "1009 16" _null_ _null_ _null_ _null_ _null_ brinoptions _null_ _null_ _null_ )); DATA(insert OID = 3801 ( brinoptions PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 17 "1009 16" _null_ _null_ _null_ _null_ _null_ brinoptions _null_ _null_ _null_ ));
DESCR("brin(internal)"); DESCR("brin(internal)");
DATA(insert OID = 3952 ( brin_summarize_new_values PGNSP PGUID 12 1 0 0 0 f f f f f f v s 1 0 23 "2205" _null_ _null_ _null_ _null_ _null_ brin_summarize_new_values _null_ _null_ _null_ )); DATA(insert OID = 3952 ( brin_summarize_new_values PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 23 "2205" _null_ _null_ _null_ _null_ _null_ brin_summarize_new_values _null_ _null_ _null_ ));
DESCR("brin: standalone scan new table pages"); DESCR("brin: standalone scan new table pages");
DATA(insert OID = 339 ( poly_same PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "604 604" _null_ _null_ _null_ _null_ _null_ poly_same _null_ _null_ _null_ )); DATA(insert OID = 339 ( poly_same PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "604 604" _null_ _null_ _null_ _null_ _null_ poly_same _null_ _null_ _null_ ));
...@@ -2920,9 +2920,9 @@ DATA(insert OID = 2274 ( pg_stat_reset PGNSP PGUID 12 1 0 0 0 f f f f f f v ...@@ -2920,9 +2920,9 @@ DATA(insert OID = 2274 ( pg_stat_reset PGNSP PGUID 12 1 0 0 0 f f f f f f v
DESCR("statistics: reset collected statistics for current database"); DESCR("statistics: reset collected statistics for current database");
DATA(insert OID = 3775 ( pg_stat_reset_shared PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 2278 "25" _null_ _null_ _null_ _null_ _null_ pg_stat_reset_shared _null_ _null_ _null_ )); DATA(insert OID = 3775 ( pg_stat_reset_shared PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 2278 "25" _null_ _null_ _null_ _null_ _null_ pg_stat_reset_shared _null_ _null_ _null_ ));
DESCR("statistics: reset collected statistics shared across the cluster"); DESCR("statistics: reset collected statistics shared across the cluster");
DATA(insert OID = 3776 ( pg_stat_reset_single_table_counters PGNSP PGUID 12 1 0 0 0 f f f f f f v s 1 0 2278 "26" _null_ _null_ _null_ _null_ _null_ pg_stat_reset_single_table_counters _null_ _null_ _null_ )); DATA(insert OID = 3776 ( pg_stat_reset_single_table_counters PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 2278 "26" _null_ _null_ _null_ _null_ _null_ pg_stat_reset_single_table_counters _null_ _null_ _null_ ));
DESCR("statistics: reset collected statistics for a single table or index in the current database"); DESCR("statistics: reset collected statistics for a single table or index in the current database");
DATA(insert OID = 3777 ( pg_stat_reset_single_function_counters PGNSP PGUID 12 1 0 0 0 f f f f f f v s 1 0 2278 "26" _null_ _null_ _null_ _null_ _null_ pg_stat_reset_single_function_counters _null_ _null_ _null_ )); DATA(insert OID = 3777 ( pg_stat_reset_single_function_counters PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 2278 "26" _null_ _null_ _null_ _null_ _null_ pg_stat_reset_single_function_counters _null_ _null_ _null_ ));
DESCR("statistics: reset collected statistics for a single function in the current database"); DESCR("statistics: reset collected statistics for a single function in the current database");
DATA(insert OID = 3163 ( pg_trigger_depth PGNSP PGUID 12 1 0 0 0 f f f f t f s s 0 0 23 "" _null_ _null_ _null_ _null_ _null_ pg_trigger_depth _null_ _null_ _null_ )); DATA(insert OID = 3163 ( pg_trigger_depth PGNSP PGUID 12 1 0 0 0 f f f f t f s s 0 0 23 "" _null_ _null_ _null_ _null_ _null_ pg_trigger_depth _null_ _null_ _null_ ));
...@@ -5203,13 +5203,13 @@ DATA(insert OID = 3473 ( spg_range_quad_leaf_consistent PGNSP PGUID 12 1 0 0 0 ...@@ -5203,13 +5203,13 @@ DATA(insert OID = 3473 ( spg_range_quad_leaf_consistent PGNSP PGUID 12 1 0 0 0
DESCR("SP-GiST support for quad tree over range"); DESCR("SP-GiST support for quad tree over range");
/* replication slots */ /* replication slots */
DATA(insert OID = 3779 ( pg_create_physical_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f f f v u 2 0 2249 "19 16" "{19,16,19,3220}" "{i,i,o,o}" "{slot_name,immediately_reserve,slot_name,xlog_position}" _null_ _null_ pg_create_physical_replication_slot _null_ _null_ _null_ )); DATA(insert OID = 3779 ( pg_create_physical_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f t f v u 2 0 2249 "19 16" "{19,16,19,3220}" "{i,i,o,o}" "{slot_name,immediately_reserve,slot_name,xlog_position}" _null_ _null_ pg_create_physical_replication_slot _null_ _null_ _null_ ));
DESCR("create a physical replication slot"); DESCR("create a physical replication slot");
DATA(insert OID = 3780 ( pg_drop_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f f f v u 1 0 2278 "19" _null_ _null_ _null_ _null_ _null_ pg_drop_replication_slot _null_ _null_ _null_ )); DATA(insert OID = 3780 ( pg_drop_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f t f v u 1 0 2278 "19" _null_ _null_ _null_ _null_ _null_ pg_drop_replication_slot _null_ _null_ _null_ ));
DESCR("drop a replication slot"); DESCR("drop a replication slot");
DATA(insert OID = 3781 ( pg_get_replication_slots PGNSP PGUID 12 1 10 0 0 f f f f f t s s 0 0 2249 "" "{19,19,25,26,16,23,28,28,3220,3220}" "{o,o,o,o,o,o,o,o,o,o}" "{slot_name,plugin,slot_type,datoid,active,active_pid,xmin,catalog_xmin,restart_lsn,confirmed_flush_lsn}" _null_ _null_ pg_get_replication_slots _null_ _null_ _null_ )); DATA(insert OID = 3781 ( pg_get_replication_slots PGNSP PGUID 12 1 10 0 0 f f f f f t s s 0 0 2249 "" "{19,19,25,26,16,23,28,28,3220,3220}" "{o,o,o,o,o,o,o,o,o,o}" "{slot_name,plugin,slot_type,datoid,active,active_pid,xmin,catalog_xmin,restart_lsn,confirmed_flush_lsn}" _null_ _null_ pg_get_replication_slots _null_ _null_ _null_ ));
DESCR("information about replication slots currently in use"); DESCR("information about replication slots currently in use");
DATA(insert OID = 3786 ( pg_create_logical_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f f f v u 2 0 2249 "19 19" "{19,19,25,3220}" "{i,i,o,o}" "{slot_name,plugin,slot_name,xlog_position}" _null_ _null_ pg_create_logical_replication_slot _null_ _null_ _null_ )); DATA(insert OID = 3786 ( pg_create_logical_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f t f v u 2 0 2249 "19 19" "{19,19,25,3220}" "{i,i,o,o}" "{slot_name,plugin,slot_name,xlog_position}" _null_ _null_ pg_create_logical_replication_slot _null_ _null_ _null_ ));
DESCR("set up a logical replication slot"); DESCR("set up a logical replication slot");
DATA(insert OID = 3782 ( pg_logical_slot_get_changes PGNSP PGUID 12 1000 1000 25 0 f f f f f t v u 4 0 2249 "19 3220 23 1009" "{19,3220,23,1009,3220,28,25}" "{i,i,i,v,o,o,o}" "{slot_name,upto_lsn,upto_nchanges,options,location,xid,data}" _null_ _null_ pg_logical_slot_get_changes _null_ _null_ _null_ )); DATA(insert OID = 3782 ( pg_logical_slot_get_changes PGNSP PGUID 12 1000 1000 25 0 f f f f f t v u 4 0 2249 "19 3220 23 1009" "{19,3220,23,1009,3220,28,25}" "{i,i,i,v,o,o,o}" "{slot_name,upto_lsn,upto_nchanges,options,location,xid,data}" _null_ _null_ pg_logical_slot_get_changes _null_ _null_ _null_ ));
DESCR("get changes from replication slot"); DESCR("get changes from replication slot");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment