Movatterモバイル変換


[0]ホーム

URL:


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

Sign up
Appearance settings

Commit78b7aaa

Browse files
committed
Clean up some lack-of-STRICT issues in the core code, too.
A scan for missed proisstrict markings in the core code turned upthese functions:brin_summarize_new_valuespg_stat_reset_single_table_counterspg_stat_reset_single_function_counterspg_create_logical_replication_slotpg_create_physical_replication_slotpg_drop_replication_slotThe first three of these take OID, so a null argument will normally looklike a zero to them, resulting in "ERROR: could not open relation with OID0" for brin_summarize_new_values, and no action for the pg_stat_reset_XXXfunctions. The other three will dump core on a null argument, though thisis mitigated by the fact that they won't do so until after checking thatthe caller is superuser or has rolreplication privilege.In addition, the pg_logical_slot_get/peek[_binary]_changes family wasintentionally marked nonstrict, but failed to make nullness checks on allthe arguments; so again a null-pointer-dereference crash is possible butonly for superusers and rolreplication users.Add the missing ARGISNULL checks to the latter functions, and mark theformer functions as strict in pg_proc. Make that change in the backbranches too, even though we can't force initdb there, just so thatinstallations initdb'd in future won't have the issue. Since none of thesebugs rise to the level of security issues (and indeed the pg_stat_reset_XXXfunctions hardly misbehave at all), it seems sufficient to do this.In addition, fix some order-of-operations oddities in the slot_get_changesfamily, mostly cosmetic, but not the part that moves the function's lastfew operations into the PG_TRY block. As it stood, there was significantrisk for an error to exit without clearing historical information fromthe 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 activebranches, as well.
1 parentacbdda4 commit78b7aaa

File tree

2 files changed

+45
-47
lines changed

2 files changed

+45
-47
lines changed

‎src/backend/replication/logical/logicalfuncs.c

Lines changed: 40 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -274,25 +274,31 @@ logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
274274
staticDatum
275275
pg_logical_slot_get_changes_guts(FunctionCallInfofcinfo,boolconfirm,boolbinary)
276276
{
277-
Namename=PG_GETARG_NAME(0);
277+
Namename;
278278
XLogRecPtrupto_lsn;
279279
int32upto_nchanges;
280-
281280
ReturnSetInfo*rsinfo= (ReturnSetInfo*)fcinfo->resultinfo;
282281
MemoryContextper_query_ctx;
283282
MemoryContextoldcontext;
284-
285283
XLogRecPtrend_of_wal;
286284
XLogRecPtrstartptr;
287-
288285
LogicalDecodingContext*ctx;
289-
290286
ResourceOwnerold_resowner=CurrentResourceOwner;
291287
ArrayType*arr;
292288
Sizendim;
293289
List*options=NIL;
294290
DecodingOutputState*p;
295291

292+
check_permissions();
293+
294+
CheckLogicalDecodingRequirements();
295+
296+
if (PG_ARGISNULL(0))
297+
ereport(ERROR,
298+
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
299+
errmsg("slot name must not be null")));
300+
name=PG_GETARG_NAME(0);
301+
296302
if (PG_ARGISNULL(1))
297303
upto_lsn=InvalidXLogRecPtr;
298304
else
@@ -303,6 +309,12 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
303309
else
304310
upto_nchanges=PG_GETARG_INT32(2);
305311

312+
if (PG_ARGISNULL(3))
313+
ereport(ERROR,
314+
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
315+
errmsg("options array must not be null")));
316+
arr=PG_GETARG_ARRAYTYPE_P(3);
317+
306318
/* check to see if caller supports us returning a tuplestore */
307319
if (rsinfo==NULL|| !IsA(rsinfo,ReturnSetInfo))
308320
ereport(ERROR,
@@ -322,16 +334,11 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
322334
if (get_call_result_type(fcinfo,NULL,&p->tupdesc)!=TYPEFUNC_COMPOSITE)
323335
elog(ERROR,"return type must be a row type");
324336

325-
check_permissions();
326-
327-
CheckLogicalDecodingRequirements();
328-
329-
arr=PG_GETARG_ARRAYTYPE_P(3);
330-
ndim=ARR_NDIM(arr);
331-
332337
per_query_ctx=rsinfo->econtext->ecxt_per_query_memory;
333338
oldcontext=MemoryContextSwitchTo(per_query_ctx);
334339

340+
/* Deconstruct options array */
341+
ndim=ARR_NDIM(arr);
335342
if (ndim>1)
336343
{
337344
ereport(ERROR,
@@ -380,7 +387,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
380387
else
381388
end_of_wal=GetXLogReplayRecPtr(NULL);
382389

383-
CheckLogicalDecodingRequirements();
384390
ReplicationSlotAcquire(NameStr(*name));
385391

386392
PG_TRY();
@@ -442,6 +448,23 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
442448
break;
443449
CHECK_FOR_INTERRUPTS();
444450
}
451+
452+
tuplestore_donestoring(tupstore);
453+
454+
CurrentResourceOwner=old_resowner;
455+
456+
/*
457+
* Next time, start where we left off. (Hunting things, the family
458+
* business..)
459+
*/
460+
if (ctx->reader->EndRecPtr!=InvalidXLogRecPtr&&confirm)
461+
LogicalConfirmReceivedLocation(ctx->reader->EndRecPtr);
462+
463+
/* free context, call shutdown callback */
464+
FreeDecodingContext(ctx);
465+
466+
ReplicationSlotRelease();
467+
InvalidateSystemCaches();
445468
}
446469
PG_CATCH();
447470
{
@@ -452,23 +475,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
452475
}
453476
PG_END_TRY();
454477

455-
tuplestore_donestoring(tupstore);
456-
457-
CurrentResourceOwner=old_resowner;
458-
459-
/*
460-
* Next time, start where we left off. (Hunting things, the family
461-
* business..)
462-
*/
463-
if (ctx->reader->EndRecPtr!=InvalidXLogRecPtr&&confirm)
464-
LogicalConfirmReceivedLocation(ctx->reader->EndRecPtr);
465-
466-
/* free context, call shutdown callback */
467-
FreeDecodingContext(ctx);
468-
469-
ReplicationSlotRelease();
470-
InvalidateSystemCaches();
471-
472478
return (Datum)0;
473479
}
474480

@@ -478,9 +484,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
478484
Datum
479485
pg_logical_slot_get_changes(PG_FUNCTION_ARGS)
480486
{
481-
Datumret=pg_logical_slot_get_changes_guts(fcinfo, true, false);
482-
483-
returnret;
487+
returnpg_logical_slot_get_changes_guts(fcinfo, true, false);
484488
}
485489

486490
/*
@@ -489,9 +493,7 @@ pg_logical_slot_get_changes(PG_FUNCTION_ARGS)
489493
Datum
490494
pg_logical_slot_peek_changes(PG_FUNCTION_ARGS)
491495
{
492-
Datumret=pg_logical_slot_get_changes_guts(fcinfo, false, false);
493-
494-
returnret;
496+
returnpg_logical_slot_get_changes_guts(fcinfo, false, false);
495497
}
496498

497499
/*
@@ -500,9 +502,7 @@ pg_logical_slot_peek_changes(PG_FUNCTION_ARGS)
500502
Datum
501503
pg_logical_slot_get_binary_changes(PG_FUNCTION_ARGS)
502504
{
503-
Datumret=pg_logical_slot_get_changes_guts(fcinfo, true, true);
504-
505-
returnret;
505+
returnpg_logical_slot_get_changes_guts(fcinfo, true, true);
506506
}
507507

508508
/*
@@ -511,7 +511,5 @@ pg_logical_slot_get_binary_changes(PG_FUNCTION_ARGS)
511511
Datum
512512
pg_logical_slot_peek_binary_changes(PG_FUNCTION_ARGS)
513513
{
514-
Datumret=pg_logical_slot_get_changes_guts(fcinfo, false, true);
515-
516-
returnret;
514+
returnpg_logical_slot_get_changes_guts(fcinfo, false, true);
517515
}

‎src/include/catalog/pg_proc.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2812,9 +2812,9 @@ DATA(insert OID = 2274 ( pg_stat_resetPGNSP PGUID 12 1 0 0 0 f f f f f f v
28122812
DESCR("statistics: reset collected statistics for current database");
28132813
DATA(insertOID=3775 (pg_stat_reset_sharedPGNSPPGUID121000fffftfv102278"25"_null__null__null__null_pg_stat_reset_shared_null__null__null_ ));
28142814
DESCR("statistics: reset collected statistics shared across the cluster");
2815-
DATA(insertOID=3776 (pg_stat_reset_single_table_countersPGNSPPGUID121000ffffffv102278"26"_null__null__null__null_pg_stat_reset_single_table_counters_null__null__null_ ));
2815+
DATA(insertOID=3776 (pg_stat_reset_single_table_countersPGNSPPGUID121000fffftfv102278"26"_null__null__null__null_pg_stat_reset_single_table_counters_null__null__null_ ));
28162816
DESCR("statistics: reset collected statistics for a single table or index in the current database");
2817-
DATA(insertOID=3777 (pg_stat_reset_single_function_countersPGNSPPGUID121000ffffffv102278"26"_null__null__null__null_pg_stat_reset_single_function_counters_null__null__null_ ));
2817+
DATA(insertOID=3777 (pg_stat_reset_single_function_countersPGNSPPGUID121000fffftfv102278"26"_null__null__null__null_pg_stat_reset_single_function_counters_null__null__null_ ));
28182818
DESCR("statistics: reset collected statistics for a single function in the current database");
28192819

28202820
DATA(insertOID=3163 (pg_trigger_depthPGNSPPGUID121000fffftfs0023""_null__null__null__null_pg_trigger_depth_null__null__null_ ));
@@ -4956,13 +4956,13 @@ DATA(insert OID = 3473 ( spg_range_quad_leaf_consistentPGNSP PGUID 12 1 0 0 0
49564956
DESCR("SP-GiST support for quad tree over range");
49574957

49584958
/* replication slots */
4959-
DATA(insertOID=3779 (pg_create_physical_replication_slotPGNSPPGUID121000ffffffv102249"19""{19,19,3220}""{i,o,o}""{slot_name,slot_name,xlog_position}"_null_pg_create_physical_replication_slot_null__null__null_ ));
4959+
DATA(insertOID=3779 (pg_create_physical_replication_slotPGNSPPGUID121000fffftfv102249"19""{19,19,3220}""{i,o,o}""{slot_name,slot_name,xlog_position}"_null_pg_create_physical_replication_slot_null__null__null_ ));
49604960
DESCR("create a physical replication slot");
4961-
DATA(insertOID=3780 (pg_drop_replication_slotPGNSPPGUID121000ffffffv102278"19"_null__null__null__null_pg_drop_replication_slot_null__null__null_ ));
4961+
DATA(insertOID=3780 (pg_drop_replication_slotPGNSPPGUID121000fffftfv102278"19"_null__null__null__null_pg_drop_replication_slot_null__null__null_ ));
49624962
DESCR("drop a replication slot");
49634963
DATA(insertOID=3781 (pg_get_replication_slotsPGNSPPGUID1211000fffffts002249"""{19,19,25,26,16,28,28,3220}""{o,o,o,o,o,o,o,o}""{slot_name,plugin,slot_type,datoid,active,xmin,catalog_xmin,restart_lsn}"_null_pg_get_replication_slots_null__null__null_ ));
49644964
DESCR("information about replication slots currently in use");
4965-
DATA(insertOID=3786 (pg_create_logical_replication_slotPGNSPPGUID121000ffffffv202249"19 19""{19,19,25,3220}""{i,i,o,o}""{slot_name,plugin,slot_name,xlog_position}"_null_pg_create_logical_replication_slot_null__null__null_ ));
4965+
DATA(insertOID=3786 (pg_create_logical_replication_slotPGNSPPGUID121000fffftfv202249"19 19""{19,19,25,3220}""{i,i,o,o}""{slot_name,plugin,slot_name,xlog_position}"_null_pg_create_logical_replication_slot_null__null__null_ ));
49664966
DESCR("set up a logical replication slot");
49674967
DATA(insertOID=3782 (pg_logical_slot_get_changesPGNSPPGUID1210001000250ffffftv402249"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_pg_logical_slot_get_changes_null__null__null_ ));
49684968
DESCR("get changes from replication slot");

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp