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

Commit34486b6

Browse files
committed
Exclude parallel workers from connection privilege/limit checks.
Cause parallel workers to not check datallowconn, rolcanlogin, andACL_CONNECT privileges. The leader already checked these things(except for rolcanlogin which might have been checked for a differentrole). Re-checking can accomplish little except to induce unexpectedfailures in applications that might not even be aware that their queryhas been parallelized. We already had the principle that parallelworkers rely on their leader to pass a valid set of authorizationinformation, so this change just extends that a bit further.Also, modify the ReservedConnections, datconnlimit and rolconnlimitlogic so that these limits are only enforced against regular backends,and only regular backends are counted while checking if the limitswere already reached. Previously, background processes that had anassigned database or role were subject to these limits (with ratherrandom exclusions for autovac workers and walsenders), and the set ofexisting processes that counted against each limit was quite haphazardas well. The point of these limits, AFAICS, is to ensure theavailability of PGPROC slots for regular backends. Since all othertypes of processes have their own separate pools of PGPROC slots, itmakes no sense either to enforce these limits against them or to countthem while enforcing the limit.While edge-case failures of these sorts have been possible for along time, the problem got a good deal worse with commit5a2fed9(CVE-2024-10978), which caused parallel workers to make some of thesechecks using the leader's current role where before we had used itsAuthenticatedUserId, thus allowing parallel queries to fail afterSET ROLE. The previous behavior was fairly accidental and I haveno desire to return to it.This patch includes reverting73c9f91, which was an emergency hackto suppress these same checks in some cases. It wasn't complete,as shown by a recent bug report from Laurenz Albe. We can also revertfd4d93d and4922173, which hacked around the same problems in oneregression test.In passing, remove the special case for autovac workers inCheckMyDatabase; it seems cleaner to have AutoVacWorkerMain passthe INIT_PG_OVERRIDE_ALLOW_CONNS flag, now that that does what'sneeded.Like5a2fed9, back-patch to supported branches (which sadly nolonger includes v12).Discussion:https://postgr.es/m/1808397.1735156190@sss.pgh.pa.us
1 parent2bdf1b2 commit34486b6

File tree

11 files changed

+80
-92
lines changed

11 files changed

+80
-92
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,10 +1419,16 @@ ParallelWorkerMain(Datum main_arg)
14191419
fps->session_user_is_superuser);
14201420
SetCurrentRoleId(fps->outer_user_id,fps->role_is_superuser);
14211421

1422-
/* Restore database connection. */
1422+
/*
1423+
* Restore database connection. We skip connection authorization checks,
1424+
* reasoning that (a) the leader checked these things when it started, and
1425+
* (b) we do not want parallel mode to cause these failures, because that
1426+
* would make use of parallel query plans not transparent to applications.
1427+
*/
14231428
BackgroundWorkerInitializeConnectionByOid(fps->database_id,
14241429
fps->authenticated_user_id,
1425-
0);
1430+
BGWORKER_BYPASS_ALLOWCONN |
1431+
BGWORKER_BYPASS_ROLELOGINCHECK);
14261432

14271433
/*
14281434
* Set the client encoding to the database encoding, since that is what

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
466466
proc->databaseId=databaseid;
467467
proc->roleId=owner;
468468
proc->tempNamespaceId=InvalidOid;
469-
proc->isBackgroundWorker=false;
469+
proc->isBackgroundWorker=true;
470470
proc->lwWaiting=LW_WS_NOT_WAITING;
471471
proc->lwWaitMode=0;
472472
proc->waitLock=NULL;

‎src/backend/postmaster/autovacuum.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,12 +1551,16 @@ AutoVacWorkerMain(char *startup_data, size_t startup_data_len)
15511551
pgstat_report_autovac(dbid);
15521552

15531553
/*
1554-
* Connect to the selected database, specifying no particular user
1554+
* Connect to the selected database, specifying no particular user,
1555+
* and ignoring datallowconn. Collect the database's name for
1556+
* display.
15551557
*
15561558
* Note: if we have selected a just-deleted database (due to using
15571559
* stale stats info), we'll fail and exit here.
15581560
*/
1559-
InitPostgres(NULL,dbid,NULL,InvalidOid,0,dbname);
1561+
InitPostgres(NULL,dbid,NULL,InvalidOid,
1562+
INIT_PG_OVERRIDE_ALLOW_CONNS,
1563+
dbname);
15601564
SetProcessingMode(NormalProcessing);
15611565
set_ps_display(dbname);
15621566
ereport(DEBUG1,

‎src/backend/postmaster/bgworker.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,7 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username, u
854854
BackgroundWorker*worker=MyBgworkerEntry;
855855
bits32init_flags=0;/* never honor session_preload_libraries */
856856

857-
/* ignore datallowconn? */
857+
/* ignore datallowconn and ACL_CONNECT? */
858858
if (flags&BGWORKER_BYPASS_ALLOWCONN)
859859
init_flags |=INIT_PG_OVERRIDE_ALLOW_CONNS;
860860
/* ignore rolcanlogin? */
@@ -888,7 +888,7 @@ BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
888888
BackgroundWorker*worker=MyBgworkerEntry;
889889
bits32init_flags=0;/* never honor session_preload_libraries */
890890

891-
/* ignore datallowconn? */
891+
/* ignore datallowconn and ACL_CONNECT? */
892892
if (flags&BGWORKER_BYPASS_ALLOWCONN)
893893
init_flags |=INIT_PG_OVERRIDE_ALLOW_CONNS;
894894
/* ignore rolcanlogin? */

‎src/backend/storage/ipc/procarray.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3622,8 +3622,7 @@ CountDBBackends(Oid databaseid)
36223622
}
36233623

36243624
/*
3625-
* CountDBConnections --- counts database backends ignoring any background
3626-
*worker processes
3625+
* CountDBConnections --- counts database backends (only regular backends)
36273626
*/
36283627
int
36293628
CountDBConnections(Oiddatabaseid)
@@ -3695,6 +3694,7 @@ CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conflictPending)
36953694

36963695
/*
36973696
* CountUserBackends --- count backends that are used by specified user
3697+
* (only regular backends, not any type of background worker)
36983698
*/
36993699
int
37003700
CountUserBackends(Oidroleid)

‎src/backend/storage/lmgr/proc.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ InitProcess(void)
432432
MyProc->databaseId=InvalidOid;
433433
MyProc->roleId=InvalidOid;
434434
MyProc->tempNamespaceId=InvalidOid;
435-
MyProc->isBackgroundWorker=AmBackgroundWorkerProcess();
435+
MyProc->isBackgroundWorker=!AmRegularBackendProcess();
436436
MyProc->delayChkptFlags=0;
437437
MyProc->statusFlags=0;
438438
/* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */
@@ -631,7 +631,7 @@ InitAuxiliaryProcess(void)
631631
MyProc->databaseId=InvalidOid;
632632
MyProc->roleId=InvalidOid;
633633
MyProc->tempNamespaceId=InvalidOid;
634-
MyProc->isBackgroundWorker=AmBackgroundWorkerProcess();
634+
MyProc->isBackgroundWorker=true;
635635
MyProc->delayChkptFlags=0;
636636
MyProc->statusFlags=0;
637637
MyProc->lwWaiting=LW_WS_NOT_WAITING;

‎src/backend/utils/init/miscinit.c

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -755,13 +755,27 @@ has_rolreplication(Oid roleid)
755755
* Initialize user identity during normal backend startup
756756
*/
757757
void
758-
InitializeSessionUserId(constchar*rolename,Oidroleid,boolbypass_login_check)
758+
InitializeSessionUserId(constchar*rolename,Oidroleid,
759+
boolbypass_login_check)
759760
{
760761
HeapTupleroleTup;
761762
Form_pg_authidrform;
762763
char*rname;
763764
boolis_superuser;
764765

766+
/*
767+
* In a parallel worker, we don't have to do anything here.
768+
* ParallelWorkerMain already set our output variables, and we aren't
769+
* going to enforce either rolcanlogin or rolconnlimit. Furthermore, we
770+
* don't really want to perform a catalog lookup for the role: we don't
771+
* want to fail if it's been dropped.
772+
*/
773+
if (InitializingParallelWorker)
774+
{
775+
Assert(bypass_login_check);
776+
return;
777+
}
778+
765779
/*
766780
* Don't do scans if we're bootstrapping, none of the system catalogs
767781
* exist yet, and they should be owned by postgres anyway.
@@ -777,68 +791,52 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec
777791

778792
/*
779793
* Look up the role, either by name if that's given or by OID if not.
780-
* Normally we have to fail if we don't find it, but in parallel workers
781-
* just return without doing anything: all the critical work has been done
782-
* already. The upshot of that is that if the role has been deleted, we
783-
* will not enforce its rolconnlimit against parallel workers anymore.
784794
*/
785795
if (rolename!=NULL)
786796
{
787797
roleTup=SearchSysCache1(AUTHNAME,PointerGetDatum(rolename));
788798
if (!HeapTupleIsValid(roleTup))
789-
{
790-
if (InitializingParallelWorker)
791-
return;
792799
ereport(FATAL,
793800
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
794801
errmsg("role \"%s\" does not exist",rolename)));
795-
}
796802
}
797803
else
798804
{
799805
roleTup=SearchSysCache1(AUTHOID,ObjectIdGetDatum(roleid));
800806
if (!HeapTupleIsValid(roleTup))
801-
{
802-
if (InitializingParallelWorker)
803-
return;
804807
ereport(FATAL,
805808
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
806809
errmsg("role with OID %u does not exist",roleid)));
807-
}
808810
}
809811

810812
rform= (Form_pg_authid)GETSTRUCT(roleTup);
811813
roleid=rform->oid;
812814
rname=NameStr(rform->rolname);
813815
is_superuser=rform->rolsuper;
814816

815-
/* In a parallel worker, ParallelWorkerMain already set these variables */
816-
if (!InitializingParallelWorker)
817-
{
818-
SetAuthenticatedUserId(roleid);
817+
SetAuthenticatedUserId(roleid);
819818

820-
/*
821-
* Set SessionUserId and related variables, including "role", via the
822-
* GUC mechanisms.
823-
*
824-
* Note: ideally we would use PGC_S_DYNAMIC_DEFAULT here, so that
825-
* session_authorization could subsequently be changed from
826-
* pg_db_role_setting entries. Instead, session_authorization in
827-
* pg_db_role_setting has no effect. Changing that would require
828-
* solving two problems:
829-
*
830-
* 1. If pg_db_role_setting has values for both session_authorization
831-
* and role, we could not be sure which order those would be applied
832-
* in, and it would matter.
833-
*
834-
* 2. Sites may have years-old session_authorization entries. There's
835-
* not been any particular reason to remove them. Ending the dormancy
836-
* of those entries could seriously change application behavior, so
837-
* only a major release should do that.
838-
*/
839-
SetConfigOption("session_authorization",rname,
840-
PGC_BACKEND,PGC_S_OVERRIDE);
841-
}
819+
/*
820+
* Set SessionUserId and related variables, including "role", via the GUC
821+
* mechanisms.
822+
*
823+
* Note: ideally we would use PGC_S_DYNAMIC_DEFAULT here, so that
824+
* session_authorization could subsequently be changed from
825+
* pg_db_role_setting entries. Instead, session_authorization in
826+
* pg_db_role_setting has no effect. Changing that would require solving
827+
* two problems:
828+
*
829+
* 1. If pg_db_role_setting has values for both session_authorization and
830+
* role, we could not be sure which order those would be applied in, and
831+
* it would matter.
832+
*
833+
* 2. Sites may have years-old session_authorization entries. There's not
834+
* been any particular reason to remove them. Ending the dormancy of
835+
* those entries could seriously change application behavior, so only a
836+
* major release should do that.
837+
*/
838+
SetConfigOption("session_authorization",rname,
839+
PGC_BACKEND,PGC_S_OVERRIDE);
842840

843841
/*
844842
* These next checks are not enforced when in standalone mode, so that
@@ -848,7 +846,8 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec
848846
if (IsUnderPostmaster)
849847
{
850848
/*
851-
* Is role allowed to login at all?
849+
* Is role allowed to login at all? (But background workers can
850+
* override this by setting bypass_login_check.)
852851
*/
853852
if (!bypass_login_check&& !rform->rolcanlogin)
854853
ereport(FATAL,
@@ -857,7 +856,9 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec
857856
rname)));
858857

859858
/*
860-
* Check connection limit for this role.
859+
* Check connection limit for this role. We enforce the limit only
860+
* for regular backends, since other process types have their own
861+
* PGPROC pools.
861862
*
862863
* There is a race condition here --- we create our PGPROC before
863864
* checking for other PGPROCs. If two backends did this at about the
@@ -867,6 +868,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec
867868
* just document that the connection limit is approximate.
868869
*/
869870
if (rform->rolconnlimit >=0&&
871+
AmRegularBackendProcess()&&
870872
!is_superuser&&
871873
CountUserBackends(roleid)>rform->rolconnlimit)
872874
ereport(FATAL,

‎src/backend/utils/init/postinit.c

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include"access/genam.h"
2323
#include"access/heapam.h"
2424
#include"access/htup_details.h"
25-
#include"access/parallel.h"
2625
#include"access/session.h"
2726
#include"access/tableam.h"
2827
#include"access/xact.h"
@@ -341,13 +340,13 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
341340
* These checks are not enforced when in standalone mode, so that there is
342341
* a way to recover from disabling all access to all databases, for
343342
* example "UPDATE pg_database SET datallowconn = false;".
344-
*
345-
* We do not enforce them for autovacuum worker processes either.
346343
*/
347-
if (IsUnderPostmaster&& !AmAutoVacuumWorkerProcess())
344+
if (IsUnderPostmaster)
348345
{
349346
/*
350347
* Check that the database is currently allowing connections.
348+
* (Background processes can override this test and the next one by
349+
* setting override_allow_connections.)
351350
*/
352351
if (!dbform->datallowconn&& !override_allow_connections)
353352
ereport(FATAL,
@@ -360,7 +359,7 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
360359
* is redundant, but since we have the flag, might as well check it
361360
* and save a few cycles.)
362361
*/
363-
if (!am_superuser&&
362+
if (!am_superuser&& !override_allow_connections&&
364363
object_aclcheck(DatabaseRelationId,MyDatabaseId,GetUserId(),
365364
ACL_CONNECT)!=ACLCHECK_OK)
366365
ereport(FATAL,
@@ -369,7 +368,9 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
369368
errdetail("User does not have CONNECT privilege.")));
370369

371370
/*
372-
* Check connection limit for this database.
371+
* Check connection limit for this database. We enforce the limit
372+
* only for regular backends, since other process types have their own
373+
* PGPROC pools.
373374
*
374375
* There is a race condition here --- we create our PGPROC before
375376
* checking for other PGPROCs. If two backends did this at about the
@@ -379,6 +380,7 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
379380
* just document that the connection limit is approximate.
380381
*/
381382
if (dbform->datconnlimit >=0&&
383+
AmRegularBackendProcess()&&
382384
!am_superuser&&
383385
CountDBConnections(MyDatabaseId)>dbform->datconnlimit)
384386
ereport(FATAL,
@@ -865,23 +867,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
865867
{
866868
InitializeSessionUserId(username,useroid,
867869
(flags&INIT_PG_OVERRIDE_ROLE_LOGIN)!=0);
868-
869-
/*
870-
* In a parallel worker, set am_superuser based on the
871-
* authenticated user ID, not the current role. This is pretty
872-
* dubious but it matches our historical behavior. Note that this
873-
* value of am_superuser is used only for connection-privilege
874-
* checks here and in CheckMyDatabase (we won't reach
875-
* process_startup_options in a background worker).
876-
*
877-
* In other cases, there's been no opportunity for the current
878-
* role to diverge from the authenticated user ID yet, so we can
879-
* just rely on superuser() and avoid an extra catalog lookup.
880-
*/
881-
if (InitializingParallelWorker)
882-
am_superuser=superuser_arg(GetAuthenticatedUserId());
883-
else
884-
am_superuser=superuser();
870+
am_superuser=superuser();
885871
}
886872
}
887873
else
@@ -908,17 +894,16 @@ InitPostgres(const char *in_dbname, Oid dboid,
908894
}
909895

910896
/*
911-
* The last few connection slots are reserved for superusers and roles
912-
* with privileges of pg_use_reserved_connections. Replication
913-
* connections are drawn from slots reserved with max_wal_senders and are
914-
* not limited by max_connections, superuser_reserved_connections, or
915-
* reserved_connections.
897+
* The last few regular connection slots are reserved for superusers and
898+
* roles with privileges of pg_use_reserved_connections. We do not apply
899+
* these limits to background processes, since they all have their own
900+
* pools of PGPROC slots.
916901
*
917902
* Note: At this point, the new backend has already claimed a proc struct,
918903
* so we must check whether the number of free slots is strictly less than
919904
* the reserved connection limits.
920905
*/
921-
if (!am_superuser&& !am_walsender&&
906+
if (AmRegularBackendProcess()&& !am_superuser&&
922907
(SuperuserReservedConnections+ReservedConnections)>0&&
923908
!HaveNFreeProcs(SuperuserReservedConnections+ReservedConnections,&nfree))
924909
{

‎src/include/miscadmin.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ typedef enum BackendType
376376

377377
externPGDLLIMPORTBackendTypeMyBackendType;
378378

379+
#defineAmRegularBackendProcess()(MyBackendType == B_BACKEND)
379380
#defineAmAutoVacuumLauncherProcess() (MyBackendType == B_AUTOVAC_LAUNCHER)
380381
#defineAmAutoVacuumWorkerProcess()(MyBackendType == B_AUTOVAC_WORKER)
381382
#defineAmBackgroundWorkerProcess() (MyBackendType == B_BG_WORKER)

‎src/include/storage/proc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ struct PGPROC
216216
OidtempNamespaceId;/* OID of temp schema this backend is
217217
* using */
218218

219-
boolisBackgroundWorker;/* true ifbackground worker. */
219+
boolisBackgroundWorker;/* true ifnot a regular backend. */
220220

221221
/*
222222
* While in hot standby mode, shows that a conflict signal has been sent

‎src/test/modules/worker_spi/worker_spi.c

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,6 @@ worker_spi_main(Datum main_arg)
169169
BackgroundWorkerInitializeConnection(worker_spi_database,
170170
worker_spi_role,flags);
171171

172-
/*
173-
* Disable parallel query for workers started with
174-
* BGWORKER_BYPASS_ALLOWCONN or BGWORKER_BYPASS_ROLELOGINCHECK so as these
175-
* don't attempt connections using a database or a role that may not allow
176-
* that.
177-
*/
178-
if ((flags& (BGWORKER_BYPASS_ALLOWCONN |BGWORKER_BYPASS_ROLELOGINCHECK)))
179-
SetConfigOption("max_parallel_workers_per_gather","0",
180-
PGC_USERSET,PGC_S_OVERRIDE);
181-
182172
elog(LOG,"%s initialized with %s.%s",
183173
MyBgworkerEntry->bgw_name,table->schema,table->name);
184174
initialize_worker_spi(table);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp