Hi, On Wed, May 13, 2026 at 04:20:21PM -0400, Robert Haas wrote: > On Tue, Apr 28, 2026 at 7:17 AM Bertrand Drouvot > <[email protected]> wrote: > > 0003: Add Assert guard to detect permission check before lock regressions > > > > Add instrumentation under USE_ASSERT_CHECKING to detect cases where > > object_aclcheck() > > is called on a referenced object before a lock is held on it, which would > > widen > > the TOCTOU window between the permission check and the dependency recording. > > I really like the idea of having some kind of cross-check system that > can detect future (or current) coding mistakes.
Thanks for the feedback! BTW, it detected a new one due to 4793fc41f82, so v21 attached does fix it to make the CI green. > But what I wonder > about this mechanism is: should we instead be insisting that we take a > lock and check permissions on every dependency? Is it an error to > record a dependency on an object without any sort of permissions > check? I'm not sure. For example, currently, without execute privilege on myfunc(), one could create a view like: CREATE VIEW v1 AS SELECT myfunc(a) FROM t1; so that the dependency is recorded. The execution permission is checked when the view is queried. I don't think this example is a bug, so that I'm doubtful about insisting that we take a lock and check permissions on every dependency. > Also, I think the mechanism might not be entirely safe. ProcessUtility > can result in executing user-defined functions which could > theoretically run other DDL and then it seems like this code would get > confused. The assert fires when an aclcheck was tracked and the lock is not held. Since the locks are transaction-scoped, any lock acquired during the statement persists, so the assert passes regardless of nesting. So that looks not possible to get false positives (assert fires) due to user-defined functions running other DDL. The concern would be false negatives (missed detection) due to the reset wiping the outer DDL's entries. I believe, that's theoretically possible only if: - The outer DDL has a P1 bug (aclcheck without lock on object X) - A user-defined function happens to run DDL that also aclchecks and locks the same object X - The user-defined function DDL's lock satisfies the assert for the outer DDL But then, the bug would be caught in normal testing without the user-defined function being present. So I'm not sure how this code could get confused. Do you have an example of what you have in mind? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 3732333ec27f6a5c124520847adf545516f65360 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Mon, 27 Apr 2026 14:01:46 +0000 Subject: [PATCH v21 1/3] Avoid orphaned objects dependencies Concurrent DDL can create orphaned dependencies in pg_depend, objects referencing other objects that no longer exist. For example: Scenario 1: session 1: begin; drop schema schem; session 2: create a function in the schema schem session 1: commit; With the above, the function created in session 2 would be linked to a non existing schema. Scenario 2: session 1: begin; create a function in the schema schem session 2: drop schema schem; session 1: commit; With the above, the function created in session 1 would be linked to a non existing schema. Fix by acquiring AccessShareLock on referenced objects when recording dependencies. This conflicts with AccessExclusiveLock taken by DROP, preventing the race. After acquiring the lock, verify the object still exists, if it was dropped concurrently, report an error. The lock and check is done in both recordMultipleDependencies() and changeDependencyFor(). The patch adds a few tests for some dependency cases (that would currently produce orphaned objects): - schema and function (as the above scenarios) - alter a dependency (function and schema) - function and arg type - function and return type - function and function - domain and domain - table and type - server and foreign data wrapper Author: Bertrand Drouvot <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] --- src/backend/catalog/dependency.c | 72 ++++++++++ src/backend/catalog/objectaddress.c | 65 +++++++++ src/backend/catalog/pg_depend.c | 16 ++- src/backend/utils/errcodes.txt | 1 + src/include/catalog/dependency.h | 2 + src/include/catalog/objectaddress.h | 1 + .../expected/test_dependencies_locks.out | 129 ++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/test_dependencies_locks.spec | 96 +++++++++++++ src/test/regress/expected/alter_table.out | 11 +- 10 files changed, 386 insertions(+), 8 deletions(-) 26.4% src/backend/catalog/ 41.3% src/test/isolation/expected/ 27.7% src/test/isolation/specs/ 4.3% src/ diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index fdb8e67e1f5..7e26691393d 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -87,6 +87,7 @@ #include "parser/parsetree.h" #include "rewrite/rewriteRemove.h" #include "storage/lmgr.h" +#include "storage/lock.h" #include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -1606,6 +1607,77 @@ ReleaseDeletionLock(const ObjectAddress *object) AccessExclusiveLock); } +/* + * LockNotPinnedObject + * + * Lock the object that we are about to record a dependency on. + * After it's locked, verify that it hasn't been dropped while we + * weren't looking. If the object has been dropped, this function + * does not return! + * + * If the caller already holds a lock that conflicts with DROP + * (AccessShareLock or stronger), skip the lock acquisition entirely. + */ +void +LockNotPinnedObject(const ObjectAddress *object) +{ + if (isObjectPinned(object)) + return; + + if (object->classId == RelationRelationId) + { + /* skip shared relations as they are pinned */ + if (IsSharedRelation(object->objectId)) + return; + + /* + * We must be in one of the two following cases that would already + * prevent the relation to be dropped: 1. The relation is already + * locked (could be an existing relation or a relation that we are + * creating). 2. The relation is protected indirectly (i.e an index + * protected by a lock on its table, a table protected by a lock on a + * function that depends of the table...). To avoid any risks, acquire + * a lock if there is none. That may add unnecessary lock for 2. but + * that's worth it. + */ + if (!CheckRelationOidLockedByMe(object->objectId, AccessShareLock, true)) + LockRelationOid(object->objectId, AccessShareLock); + return; + } + else + { + LOCKTAG tag; + + SET_LOCKTAG_OBJECT(tag, + MyDatabaseId, + object->classId, + object->objectId, + 0); + + if (LockHeldByMe(&tag, AccessShareLock, true)) + return; + + /* assume we should lock the whole object not a sub-object */ + LockDatabaseObject(object->classId, object->objectId, 0, AccessShareLock); + } + + /* check if object still exists */ + if (!ObjectByIdExist(object, false)) + { + /* + * It might be possible that we are creating it (for example creating + * a composite type while creating a relation), so bypass the syscache + * lookup and use a SnapshotSelf scan instead to cover this scenario. + */ + if (!ObjectByIdExist(object, true)) + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_DOES_NOT_EXIST), + errmsg("dependent object does not exist"), + errdetail("Class OID is %u and object OID is %u", + object->classId, object->objectId))); + } +} + /* * recordDependencyOnExpr - find expression dependencies * diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 050b7829eb0..b5786532af8 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -90,6 +90,7 @@ #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/regproc.h" +#include "utils/snapmgr.h" #include "utils/syscache.h" /* @@ -2696,6 +2697,70 @@ get_object_namespace(const ObjectAddress *address) return oid; } +/* + * ObjectByIdExist + * + * Return whether the given object exists. + * + * If use_snapshot_self is false, uses the syscache (which sees committed data). + * If use_snapshot_self is true, does a direct catalog scan with SnapshotSelf + * to also see objects created in the current transaction. + */ +bool +ObjectByIdExist(const ObjectAddress *address, bool use_snapshot_self) +{ + HeapTuple tuple; + SysCacheIdentifier cache = SYSCACHEID_INVALID; + + if (!use_snapshot_self) + { + const ObjectPropertyType *property; + + property = get_object_property_data(address->classId); + cache = property->oid_catcache_id; + } + + if (cache != SYSCACHEID_INVALID) + { + tuple = SearchSysCache1(cache, ObjectIdGetDatum(address->objectId)); + + if (!HeapTupleIsValid(tuple)) + return false; + + ReleaseSysCache(tuple); + return true; + } + else + { + Relation rel; + ScanKeyData skey[1]; + SysScanDesc scan; + Snapshot snapshot; + + if (use_snapshot_self) + snapshot = SnapshotSelf; + else + snapshot = NULL; + + rel = table_open(address->classId, AccessShareLock); + + ScanKeyInit(&skey[0], + get_object_attnum_oid(address->classId), + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(address->objectId)); + + scan = systable_beginscan(rel, get_object_oid_index(address->classId), + true, snapshot, 1, skey); + + tuple = systable_getnext(scan); + + systable_endscan(scan); + table_close(rel, AccessShareLock); + + return HeapTupleIsValid(tuple); + } +} + /* * Return ObjectType for the given object type as given by * getObjectTypeDescription; if no valid ObjectType code exists, but it's a diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 07c2d41c189..5618e3d26fa 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -33,8 +33,6 @@ #include "utils/syscache.h" -static bool isObjectPinned(const ObjectAddress *object); - /* * Record a dependency between 2 objects via their respective ObjectAddress. @@ -109,6 +107,12 @@ recordMultipleDependencies(const ObjectAddress *depender, if (isObjectPinned(referenced)) continue; + /* + * Acquire a lock and check object still exists while recording the + * dependency. + */ + LockNotPinnedObject(referenced); + if (slot_init_count < max_slots) { slot[slot_stored_count] = MakeSingleTupleTableSlot(RelationGetDescr(dependDesc), @@ -507,6 +511,12 @@ changeDependencyFor(Oid classId, Oid objectId, return 1; } + /* + * Acquire a lock and check object still exists while changing the + * dependency. + */ + LockNotPinnedObject(&objAddr); + depRel = table_open(DependRelationId, RowExclusiveLock); /* There should be existing dependency record(s), so search. */ @@ -707,7 +717,7 @@ changeDependenciesOn(Oid refClassId, Oid oldRefObjectId, * The passed subId, if any, is ignored; we assume that only whole objects * are pinned (and that this implies pinning their components). */ -static bool +bool isObjectPinned(const ObjectAddress *object) { return IsPinnedObject(object->classId, object->objectId); diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt index 5b25402ebbe..58b498f68fc 100644 --- a/src/backend/utils/errcodes.txt +++ b/src/backend/utils/errcodes.txt @@ -278,6 +278,7 @@ Section: Class 2B - Dependent Privilege Descriptors Still Exist 2B000 E ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST dependent_privilege_descriptors_still_exist 2BP01 E ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST dependent_objects_still_exist +2BP02 E ERRCODE_DEPENDENT_OBJECTS_DOES_NOT_EXIST dependent_objects_does_not_exist Section: Class 2D - Invalid Transaction Termination diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h index 2f3c1eae3c7..fa508ea70c6 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -101,6 +101,8 @@ typedef struct ObjectAddresses ObjectAddresses; /* in dependency.c */ extern void AcquireDeletionLock(const ObjectAddress *object, int flags); +extern void LockNotPinnedObject(const ObjectAddress *object); +extern bool isObjectPinned(const ObjectAddress *object); extern void ReleaseDeletionLock(const ObjectAddress *object); diff --git a/src/include/catalog/objectaddress.h b/src/include/catalog/objectaddress.h index 1f965e1faef..960b7e4bfa6 100644 --- a/src/include/catalog/objectaddress.h +++ b/src/include/catalog/objectaddress.h @@ -54,6 +54,7 @@ extern void check_object_ownership(Oid roleid, Node *object, Relation relation); extern Oid get_object_namespace(const ObjectAddress *address); +extern bool ObjectByIdExist(const ObjectAddress *address, bool use_snapshot_self); extern bool is_objectclass_supported(Oid class_id); extern const char *get_object_class_descr(Oid class_id); diff --git a/src/test/isolation/expected/test_dependencies_locks.out b/src/test/isolation/expected/test_dependencies_locks.out new file mode 100644 index 00000000000..820680f5e16 --- /dev/null +++ b/src/test/isolation/expected/test_dependencies_locks.out @@ -0,0 +1,129 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_begin s1_create_function_in_schema s2_drop_schema s1_commit +step s1_begin: BEGIN; +step s1_create_function_in_schema: CREATE FUNCTION testschema.foo() RETURNS int AS 'select 1' LANGUAGE sql; +step s2_drop_schema: DROP SCHEMA testschema; <waiting ...> +step s1_commit: COMMIT; +step s2_drop_schema: <... completed> +ERROR: cannot drop schema testschema because other objects depend on it + +starting permutation: s2_begin s2_drop_schema s1_create_function_in_schema s2_commit +step s2_begin: BEGIN; +step s2_drop_schema: DROP SCHEMA testschema; +step s1_create_function_in_schema: CREATE FUNCTION testschema.foo() RETURNS int AS 'select 1' LANGUAGE sql; <waiting ...> +step s2_commit: COMMIT; +step s1_create_function_in_schema: <... completed> +ERROR: dependent object does not exist + +starting permutation: s1_begin s1_alter_function_schema s2_drop_alterschema s1_commit +step s1_begin: BEGIN; +step s1_alter_function_schema: ALTER FUNCTION public.falter() SET SCHEMA alterschema; +step s2_drop_alterschema: DROP SCHEMA alterschema; <waiting ...> +step s1_commit: COMMIT; +step s2_drop_alterschema: <... completed> +ERROR: cannot drop schema alterschema because other objects depend on it + +starting permutation: s2_begin s2_drop_alterschema s1_alter_function_schema s2_commit +step s2_begin: BEGIN; +step s2_drop_alterschema: DROP SCHEMA alterschema; +step s1_alter_function_schema: ALTER FUNCTION public.falter() SET SCHEMA alterschema; <waiting ...> +step s2_commit: COMMIT; +step s1_alter_function_schema: <... completed> +ERROR: dependent object does not exist + +starting permutation: s1_begin s1_create_function_with_argtype s2_drop_foo_type s1_commit +step s1_begin: BEGIN; +step s1_create_function_with_argtype: CREATE FUNCTION fooargtype(num foo) RETURNS int AS 'select 1' LANGUAGE sql; +step s2_drop_foo_type: DROP TYPE public.foo; <waiting ...> +step s1_commit: COMMIT; +step s2_drop_foo_type: <... completed> +ERROR: cannot drop type foo because other objects depend on it + +starting permutation: s2_begin s2_drop_foo_type s1_create_function_with_argtype s2_commit +step s2_begin: BEGIN; +step s2_drop_foo_type: DROP TYPE public.foo; +step s1_create_function_with_argtype: CREATE FUNCTION fooargtype(num foo) RETURNS int AS 'select 1' LANGUAGE sql; <waiting ...> +step s2_commit: COMMIT; +step s1_create_function_with_argtype: <... completed> +ERROR: dependent object does not exist + +starting permutation: s1_begin s1_create_function_with_rettype s2_drop_foo_rettype s1_commit +step s1_begin: BEGIN; +step s1_create_function_with_rettype: CREATE FUNCTION footrettype() RETURNS id LANGUAGE sql RETURN 1; +step s2_drop_foo_rettype: DROP DOMAIN id; <waiting ...> +step s1_commit: COMMIT; +step s2_drop_foo_rettype: <... completed> +ERROR: cannot drop type id because other objects depend on it + +starting permutation: s2_begin s2_drop_foo_rettype s1_create_function_with_rettype s2_commit +step s2_begin: BEGIN; +step s2_drop_foo_rettype: DROP DOMAIN id; +step s1_create_function_with_rettype: CREATE FUNCTION footrettype() RETURNS id LANGUAGE sql RETURN 1; <waiting ...> +step s2_commit: COMMIT; +step s1_create_function_with_rettype: <... completed> +ERROR: dependent object does not exist + +starting permutation: s1_begin s1_create_function_with_function s2_drop_function_f s1_commit +step s1_begin: BEGIN; +step s1_create_function_with_function: CREATE FUNCTION foofunc() RETURNS int LANGUAGE SQL RETURN f() + 1; +step s2_drop_function_f: DROP FUNCTION f(); <waiting ...> +step s1_commit: COMMIT; +step s2_drop_function_f: <... completed> +ERROR: cannot drop function f() because other objects depend on it + +starting permutation: s2_begin s2_drop_function_f s1_create_function_with_function s2_commit +step s2_begin: BEGIN; +step s2_drop_function_f: DROP FUNCTION f(); +step s1_create_function_with_function: CREATE FUNCTION foofunc() RETURNS int LANGUAGE SQL RETURN f() + 1; <waiting ...> +step s2_commit: COMMIT; +step s1_create_function_with_function: <... completed> +ERROR: dependent object does not exist + +starting permutation: s1_begin s1_create_domain_with_domain s2_drop_domain_id s1_commit +step s1_begin: BEGIN; +step s1_create_domain_with_domain: CREATE DOMAIN idid as id; +step s2_drop_domain_id: DROP DOMAIN id; <waiting ...> +step s1_commit: COMMIT; +step s2_drop_domain_id: <... completed> +ERROR: cannot drop type id because other objects depend on it + +starting permutation: s2_begin s2_drop_domain_id s1_create_domain_with_domain s2_commit +step s2_begin: BEGIN; +step s2_drop_domain_id: DROP DOMAIN id; +step s1_create_domain_with_domain: CREATE DOMAIN idid as id; <waiting ...> +step s2_commit: COMMIT; +step s1_create_domain_with_domain: <... completed> +ERROR: dependent object does not exist + +starting permutation: s1_begin s1_create_table_with_type s2_drop_footab_type s1_commit +step s1_begin: BEGIN; +step s1_create_table_with_type: CREATE TABLE tabtype(a footab); +step s2_drop_footab_type: DROP TYPE public.footab; <waiting ...> +step s1_commit: COMMIT; +step s2_drop_footab_type: <... completed> +ERROR: cannot drop type footab because other objects depend on it + +starting permutation: s2_begin s2_drop_footab_type s1_create_table_with_type s2_commit +step s2_begin: BEGIN; +step s2_drop_footab_type: DROP TYPE public.footab; +step s1_create_table_with_type: CREATE TABLE tabtype(a footab); <waiting ...> +step s2_commit: COMMIT; +step s1_create_table_with_type: <... completed> +ERROR: dependent object does not exist + +starting permutation: s1_begin s1_create_server_with_fdw_wrapper s2_drop_fdw_wrapper s1_commit +step s1_begin: BEGIN; +step s1_create_server_with_fdw_wrapper: CREATE SERVER srv_fdw_wrapper FOREIGN DATA WRAPPER fdw_wrapper; +step s2_drop_fdw_wrapper: DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT; <waiting ...> +step s1_commit: COMMIT; +step s2_drop_fdw_wrapper: <... completed> +ERROR: cannot drop foreign-data wrapper fdw_wrapper because other objects depend on it + +starting permutation: s2_begin s2_drop_fdw_wrapper s1_create_server_with_fdw_wrapper s2_commit +step s2_begin: BEGIN; +step s2_drop_fdw_wrapper: DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT; +step s1_create_server_with_fdw_wrapper: CREATE SERVER srv_fdw_wrapper FOREIGN DATA WRAPPER fdw_wrapper; <waiting ...> +step s2_commit: COMMIT; +step s1_create_server_with_fdw_wrapper: <... completed> +ERROR: dependent object does not exist diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 1578ba191c8..83f626d51b5 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -126,3 +126,4 @@ test: serializable-parallel-3 test: matview-write-skew test: lock-nowait test: for-portion-of +test: test_dependencies_locks \ No newline at end of file diff --git a/src/test/isolation/specs/test_dependencies_locks.spec b/src/test/isolation/specs/test_dependencies_locks.spec new file mode 100644 index 00000000000..ee4130b2dc4 --- /dev/null +++ b/src/test/isolation/specs/test_dependencies_locks.spec @@ -0,0 +1,96 @@ +# Test that concurrent DDL properly prevents orphaned dependencies. +# +# When session 1 creates an object that depends on a referenced object, +# and session 2 concurrently drops that referenced object, the lock +# acquired during dependency recording must prevent the drop or the +# create must fail with "dependent object does not exist". + +setup +{ + CREATE SCHEMA testschema; + CREATE SCHEMA alterschema; + CREATE TYPE public.foo as enum ('one', 'two'); + CREATE TYPE public.footab as enum ('three', 'four'); + CREATE DOMAIN id AS int; + CREATE FUNCTION f() RETURNS int LANGUAGE SQL RETURN 1; + CREATE FUNCTION public.falter() RETURNS int LANGUAGE SQL RETURN 1; + CREATE FOREIGN DATA WRAPPER fdw_wrapper; +} + +teardown +{ + DROP FUNCTION IF EXISTS testschema.foo(); + DROP FUNCTION IF EXISTS fooargtype(num foo); + DROP FUNCTION IF EXISTS footrettype(); + DROP FUNCTION IF EXISTS foofunc(); + DROP FUNCTION IF EXISTS public.falter(); + DROP FUNCTION IF EXISTS alterschema.falter(); + DROP DOMAIN IF EXISTS idid; + DROP SERVER IF EXISTS srv_fdw_wrapper; + DROP TABLE IF EXISTS tabtype; + DROP SCHEMA IF EXISTS testschema; + DROP SCHEMA IF EXISTS alterschema; + DROP TYPE IF EXISTS public.foo; + DROP TYPE IF EXISTS public.footab; + DROP DOMAIN IF EXISTS id; + DROP FUNCTION IF EXISTS f(); + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_wrapper; +} + +session "s1" + +step "s1_begin" { BEGIN; } +step "s1_create_function_in_schema" { CREATE FUNCTION testschema.foo() RETURNS int AS 'select 1' LANGUAGE sql; } +step "s1_create_function_with_argtype" { CREATE FUNCTION fooargtype(num foo) RETURNS int AS 'select 1' LANGUAGE sql; } +step "s1_create_function_with_rettype" { CREATE FUNCTION footrettype() RETURNS id LANGUAGE sql RETURN 1; } +step "s1_create_function_with_function" { CREATE FUNCTION foofunc() RETURNS int LANGUAGE SQL RETURN f() + 1; } +step "s1_alter_function_schema" { ALTER FUNCTION public.falter() SET SCHEMA alterschema; } +step "s1_create_domain_with_domain" { CREATE DOMAIN idid as id; } +step "s1_create_table_with_type" { CREATE TABLE tabtype(a footab); } +step "s1_create_server_with_fdw_wrapper" { CREATE SERVER srv_fdw_wrapper FOREIGN DATA WRAPPER fdw_wrapper; } +step "s1_commit" { COMMIT; } + +session "s2" + +step "s2_begin" { BEGIN; } +step "s2_drop_schema" { DROP SCHEMA testschema; } +step "s2_drop_alterschema" { DROP SCHEMA alterschema; } +step "s2_drop_foo_type" { DROP TYPE public.foo; } +step "s2_drop_foo_rettype" { DROP DOMAIN id; } +step "s2_drop_footab_type" { DROP TYPE public.footab; } +step "s2_drop_function_f" { DROP FUNCTION f(); } +step "s2_drop_domain_id" { DROP DOMAIN id; } +step "s2_drop_fdw_wrapper" { DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT; } +step "s2_commit" { COMMIT; } + +# function - schema +permutation "s1_begin" "s1_create_function_in_schema" "s2_drop_schema" "s1_commit" +permutation "s2_begin" "s2_drop_schema" "s1_create_function_in_schema" "s2_commit" + +# alter function - schema +permutation "s1_begin" "s1_alter_function_schema" "s2_drop_alterschema" "s1_commit" +permutation "s2_begin" "s2_drop_alterschema" "s1_alter_function_schema" "s2_commit" + +# function - argtype +permutation "s1_begin" "s1_create_function_with_argtype" "s2_drop_foo_type" "s1_commit" +permutation "s2_begin" "s2_drop_foo_type" "s1_create_function_with_argtype" "s2_commit" + +# function - rettype +permutation "s1_begin" "s1_create_function_with_rettype" "s2_drop_foo_rettype" "s1_commit" +permutation "s2_begin" "s2_drop_foo_rettype" "s1_create_function_with_rettype" "s2_commit" + +# function - function +permutation "s1_begin" "s1_create_function_with_function" "s2_drop_function_f" "s1_commit" +permutation "s2_begin" "s2_drop_function_f" "s1_create_function_with_function" "s2_commit" + +# domain - domain +permutation "s1_begin" "s1_create_domain_with_domain" "s2_drop_domain_id" "s1_commit" +permutation "s2_begin" "s2_drop_domain_id" "s1_create_domain_with_domain" "s2_commit" + +# table - type +permutation "s1_begin" "s1_create_table_with_type" "s2_drop_footab_type" "s1_commit" +permutation "s2_begin" "s2_drop_footab_type" "s1_create_table_with_type" "s2_commit" + +# server - foreign data wrapper +permutation "s1_begin" "s1_create_server_with_fdw_wrapper" "s2_drop_fdw_wrapper" "s1_commit" +permutation "s2_begin" "s2_drop_fdw_wrapper" "s1_create_server_with_fdw_wrapper" "s2_commit" diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 6dd22be0e8d..b891d68d4a7 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2949,11 +2949,12 @@ begin; alter table alterlock2 add constraint alterlock2nv foreign key (f1) references alterlock (f1) NOT VALID; select * from my_locks order by 1; - relname | max_lockmode -------------+----------------------- - alterlock | ShareRowExclusiveLock - alterlock2 | ShareRowExclusiveLock -(2 rows) + relname | max_lockmode +----------------+----------------------- + alterlock | ShareRowExclusiveLock + alterlock2 | ShareRowExclusiveLock + alterlock_pkey | AccessShareLock +(3 rows) commit; begin; -- 2.34.1
>From b73a9b3523a03d7d0f31616ddb0450cb691e4ce7 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Mon, 27 Apr 2026 14:19:51 +0000 Subject: [PATCH v21 2/3] Lock referenced objects before permission checks in DDL commands Add LockNotPinnedObjectById() calls before object_aclcheck() at all caller sites where a permission check on a referenced object occurs before the dependency on that object is recorded. This converts permission check before lock to lock before permission check. It closes the TOCTOU window that could allow a REVOKE to succeed between the permission check and the dependency recording. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] --- src/backend/catalog/dependency.c | 22 ++++++++++++++++++++++ src/backend/catalog/namespace.c | 1 + src/backend/catalog/pg_aggregate.c | 5 +++++ src/backend/catalog/pg_cast.c | 1 + src/backend/catalog/pg_operator.c | 2 ++ src/backend/commands/aggregatecmds.c | 2 ++ src/backend/commands/alter.c | 3 +++ src/backend/commands/collationcmds.c | 2 ++ src/backend/commands/conversioncmds.c | 3 +++ src/backend/commands/extension.c | 1 + src/backend/commands/foreigncmds.c | 5 +++++ src/backend/commands/functioncmds.c | 11 +++++++++++ src/backend/commands/indexcmds.c | 2 ++ src/backend/commands/opclasscmds.c | 2 ++ src/backend/commands/operatorcmds.c | 8 ++++++++ src/backend/commands/statscmds.c | 1 + src/backend/commands/subscriptioncmds.c | 4 ++++ src/backend/commands/tablecmds.c | 7 +++++++ src/backend/commands/trigger.c | 2 ++ src/backend/commands/tsearchcmds.c | 2 ++ src/backend/commands/typecmds.c | 9 +++++++++ src/include/catalog/dependency.h | 1 + 22 files changed, 96 insertions(+) 22.5% src/backend/catalog/ 76.2% src/backend/commands/ diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 7e26691393d..a41e6ee2175 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1678,6 +1678,24 @@ LockNotPinnedObject(const ObjectAddress *object) } } +/* + * LockNotPinnedObjectById + * + * Lock the object if it is not pinned. This is a convenience wrapper + * for callers that need to lock a referenced object before a permission + * check, converting permission check before lock to lock before permission + * check. + */ +void +LockNotPinnedObjectById(Oid classid, Oid objid) +{ + ObjectAddress object; + + ObjectAddressSet(object, classid, objid); + + LockNotPinnedObject(&object); +} + /* * recordDependencyOnExpr - find expression dependencies * @@ -1960,8 +1978,11 @@ find_expr_references_walker(Node *node, objoid = DatumGetObjectId(con->constvalue); if (SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(objoid))) + { + LockNotPinnedObjectById(ProcedureRelationId, objoid); add_object_address(ProcedureRelationId, objoid, 0, context->addrs); + } break; case REGOPEROID: case REGOPERATOROID: @@ -2057,6 +2078,7 @@ find_expr_references_walker(Node *node, { FuncExpr *funcexpr = (FuncExpr *) node; + LockNotPinnedObjectById(ProcedureRelationId, funcexpr->funcid); add_object_address(ProcedureRelationId, funcexpr->funcid, 0, context->addrs); /* fall through to examine arguments */ diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 56b87d878e8..7dec076e111 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -3512,6 +3512,7 @@ LookupCreationNamespace(const char *nspname) namespaceId = get_namespace_oid(nspname, false); + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 243b952b9cc..41b390e8d96 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -586,22 +586,26 @@ AggregateCreate(const char *aggName, */ for (i = 0; i < numArgs; i++) { + LockNotPinnedObjectById(TypeRelationId, aggArgTypes[i]); aclresult = object_aclcheck(TypeRelationId, aggArgTypes[i], GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, aggArgTypes[i]); } + LockNotPinnedObjectById(TypeRelationId, aggTransType); aclresult = object_aclcheck(TypeRelationId, aggTransType, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, aggTransType); if (OidIsValid(aggmTransType)) { + LockNotPinnedObjectById(TypeRelationId, aggmTransType); aclresult = object_aclcheck(TypeRelationId, aggmTransType, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, aggmTransType); } + LockNotPinnedObjectById(TypeRelationId, finaltype); aclresult = object_aclcheck(TypeRelationId, finaltype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, finaltype); @@ -909,6 +913,7 @@ lookup_agg_function(List *fnName, } /* Check aggregate creator has permission to call the function */ + LockNotPinnedObjectById(ProcedureRelationId, fnOid); aclresult = object_aclcheck(ProcedureRelationId, fnOid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(fnOid)); diff --git a/src/backend/catalog/pg_cast.c b/src/backend/catalog/pg_cast.c index 5119c2acda2..bb48e686f83 100644 --- a/src/backend/catalog/pg_cast.c +++ b/src/backend/catalog/pg_cast.c @@ -105,6 +105,7 @@ CastCreate(Oid sourcetypeid, Oid targettypeid, /* dependency on function */ if (OidIsValid(funcid)) { + LockNotPinnedObjectById(ProcedureRelationId, funcid); ObjectAddressSet(referenced, ProcedureRelationId, funcid); add_exact_object_address(&referenced, addrs); } diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index 6b90c774c18..a222358d081 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -654,6 +654,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId, /* not in catalogs, different from operator, so make shell */ + LockNotPinnedObjectById(NamespaceRelationId, otherNamespace); aclresult = object_aclcheck(NamespaceRelationId, otherNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) @@ -876,6 +877,7 @@ makeOperatorDependencies(HeapTuple tuple, /* Dependency on namespace */ if (OidIsValid(oper->oprnamespace)) { + LockNotPinnedObjectById(NamespaceRelationId, oper->oprnamespace); ObjectAddressSet(referenced, NamespaceRelationId, oper->oprnamespace); add_exact_object_address(&referenced, addrs); } diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c index 41b45dc6402..c2fb111daf3 100644 --- a/src/backend/commands/aggregatecmds.c +++ b/src/backend/commands/aggregatecmds.c @@ -22,6 +22,7 @@ */ #include "postgres.h" +#include "catalog/dependency.h" #include "catalog/namespace.h" #include "catalog/pg_aggregate.h" #include "catalog/pg_namespace.h" @@ -101,6 +102,7 @@ DefineAggregate(ParseState *pstate, aggNamespace = QualifiedNameGetCreationNamespace(name, &aggName); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, aggNamespace); aclresult = object_aclcheck(NamespaceRelationId, aggNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 74ceb5fe20d..03516cb0d2e 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -221,6 +221,7 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) /* User must have CREATE privilege on the namespace */ if (OidIsValid(namespaceId)) { + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) @@ -752,6 +753,7 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) NameStr(*(DatumGetName(name)))); /* User must have CREATE privilege on new namespace */ + LockNotPinnedObjectById(NamespaceRelationId, nspOid); aclresult = object_aclcheck(NamespaceRelationId, nspOid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -1014,6 +1016,7 @@ AlterObjectOwner_internal(Oid classId, Oid objectId, Oid new_ownerId) { AclResult aclresult; + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, new_ownerId, ACL_CREATE); if (aclresult != ACLCHECK_OK) diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index 0bc31ec2b6f..fa5dad1aa1f 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -21,6 +21,7 @@ #include "access/htup_details.h" #include "access/table.h" #include "access/xact.h" +#include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/namespace.h" #include "catalog/objectaccess.h" @@ -82,6 +83,7 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e collNamespace = QualifiedNameGetCreationNamespace(names, &collName); + LockNotPinnedObjectById(NamespaceRelationId, collNamespace); aclresult = object_aclcheck(NamespaceRelationId, collNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, diff --git a/src/backend/commands/conversioncmds.c b/src/backend/commands/conversioncmds.c index 5f2022d3072..d60ecdb711a 100644 --- a/src/backend/commands/conversioncmds.c +++ b/src/backend/commands/conversioncmds.c @@ -14,6 +14,7 @@ */ #include "postgres.h" +#include "catalog/dependency.h" #include "catalog/pg_conversion.h" #include "catalog/pg_namespace.h" #include "catalog/pg_proc.h" @@ -50,6 +51,7 @@ CreateConversionCommand(CreateConversionStmt *stmt) &conversion_name); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -97,6 +99,7 @@ CreateConversionCommand(CreateConversionStmt *stmt) NameListToString(func_name), "integer"))); /* Check we have EXECUTE rights for the function */ + LockNotPinnedObjectById(ProcedureRelationId, funcoid); aclresult = object_aclcheck(ProcedureRelationId, funcoid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index a330b5fd6ce..4e82a010788 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -3283,6 +3283,7 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o extensionName); /* Permission check: must have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, nspOid); aclresult = object_aclcheck(NamespaceRelationId, nspOid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, newschema); diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c index c4852be2eb2..dd31e260739 100644 --- a/src/backend/commands/foreigncmds.c +++ b/src/backend/commands/foreigncmds.c @@ -377,6 +377,7 @@ AlterForeignServerOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) check_can_set_role(GetUserId(), newOwnerId); /* New owner must have USAGE privilege on foreign-data wrapper */ + LockNotPinnedObjectById(ForeignDataWrapperRelationId, form->srvfdw); aclresult = object_aclcheck(ForeignDataWrapperRelationId, form->srvfdw, newOwnerId, ACL_USAGE); if (aclresult != ACLCHECK_OK) { @@ -997,6 +998,7 @@ CreateForeignServer(CreateForeignServerStmt *stmt) */ fdw = GetForeignDataWrapperByName(stmt->fdwname, false); + LockNotPinnedObjectById(ForeignDataWrapperRelationId, fdw->fdwid); aclresult = object_aclcheck(ForeignDataWrapperRelationId, fdw->fdwid, ownerId, ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FDW, fdw->fdwname); @@ -1188,6 +1190,7 @@ user_mapping_ddl_aclcheck(Oid umuserid, Oid serverid, const char *servername) { AclResult aclresult; + LockNotPinnedObjectById(ForeignServerRelationId, serverid); aclresult = object_aclcheck(ForeignServerRelationId, serverid, curuserid, ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, servername); @@ -1539,6 +1542,7 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid) * get the actual FDW for option validation etc. */ server = GetForeignServerByName(stmt->servername, false); + LockNotPinnedObjectById(ForeignServerRelationId, server->serverid); aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, ownerId, ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername); @@ -1598,6 +1602,7 @@ ImportForeignSchema(ImportForeignSchemaStmt *stmt) /* Check that the foreign server exists and that we have USAGE on it */ server = GetForeignServerByName(stmt->server_name, false); + LockNotPinnedObjectById(ForeignServerRelationId, server->serverid); aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername); diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 3afd762e9dc..9aec534c390 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -146,6 +146,7 @@ compute_return_type(TypeName *returnType, Oid languageOid, errdetail("Creating a shell type definition."))); namespaceId = QualifiedNameGetCreationNamespace(returnType->names, &typname); + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) @@ -158,6 +159,7 @@ compute_return_type(TypeName *returnType, Oid languageOid, CommandCounterIncrement(); } + LockNotPinnedObjectById(TypeRelationId, rettype); aclresult = object_aclcheck(TypeRelationId, rettype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, rettype); @@ -274,6 +276,7 @@ interpret_function_parameter_list(ParseState *pstate, toid = InvalidOid; /* keep compiler quiet */ } + LockNotPinnedObjectById(TypeRelationId, toid); aclresult = object_aclcheck(TypeRelationId, toid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, toid); @@ -1071,6 +1074,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt) &funcname); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -1125,6 +1129,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt) if (languageStruct->lanpltrusted) { /* if trusted language, need USAGE privilege */ + LockNotPinnedObjectById(LanguageRelationId, languageOid); aclresult = object_aclcheck(LanguageRelationId, languageOid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_LANGUAGE, @@ -1582,10 +1587,12 @@ CreateCast(CreateCastStmt *stmt) format_type_be(sourcetypeid), format_type_be(targettypeid)))); + LockNotPinnedObjectById(TypeRelationId, sourcetypeid); aclresult = object_aclcheck(TypeRelationId, sourcetypeid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, sourcetypeid); + LockNotPinnedObjectById(TypeRelationId, targettypeid); aclresult = object_aclcheck(TypeRelationId, targettypeid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, targettypeid); @@ -1874,6 +1881,7 @@ CreateTransform(CreateTransformStmt *stmt) if (!object_ownercheck(TypeRelationId, typeid, GetUserId())) aclcheck_error_type(ACLCHECK_NOT_OWNER, typeid); + LockNotPinnedObjectById(TypeRelationId, typeid); aclresult = object_aclcheck(TypeRelationId, typeid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, typeid); @@ -1883,6 +1891,7 @@ CreateTransform(CreateTransformStmt *stmt) */ langid = get_language_oid(stmt->lang, false); + LockNotPinnedObjectById(LanguageRelationId, langid); aclresult = object_aclcheck(LanguageRelationId, langid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_LANGUAGE, stmt->lang); @@ -1897,6 +1906,7 @@ CreateTransform(CreateTransformStmt *stmt) if (!object_ownercheck(ProcedureRelationId, fromsqlfuncid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION, NameListToString(stmt->fromsql->objname)); + LockNotPinnedObjectById(ProcedureRelationId, fromsqlfuncid); aclresult = object_aclcheck(ProcedureRelationId, fromsqlfuncid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, NameListToString(stmt->fromsql->objname)); @@ -1923,6 +1933,7 @@ CreateTransform(CreateTransformStmt *stmt) if (!object_ownercheck(ProcedureRelationId, tosqlfuncid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION, NameListToString(stmt->tosql->objname)); + LockNotPinnedObjectById(ProcedureRelationId, tosqlfuncid); aclresult = object_aclcheck(ProcedureRelationId, tosqlfuncid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, NameListToString(stmt->tosql->objname)); diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 9ab74c8df0a..6216ab2a5f5 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -25,6 +25,7 @@ #include "access/tableam.h" #include "access/xact.h" #include "catalog/catalog.h" +#include "catalog/dependency.h" #include "catalog/index.h" #include "catalog/indexing.h" #include "catalog/namespace.h" @@ -770,6 +771,7 @@ DefineIndex(ParseState *pstate, { AclResult aclresult; + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, root_save_userid, ACL_CREATE); if (aclresult != ACLCHECK_OK) diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c index 7493a9ccc06..ad71ee53cb2 100644 --- a/src/backend/commands/opclasscmds.c +++ b/src/backend/commands/opclasscmds.c @@ -363,6 +363,7 @@ DefineOpClass(CreateOpClassStmt *stmt) &opcname); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, namespaceoid); aclresult = object_aclcheck(NamespaceRelationId, namespaceoid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -801,6 +802,7 @@ DefineOpFamily(CreateOpFamilyStmt *stmt) &opfname); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, namespaceoid); aclresult = object_aclcheck(NamespaceRelationId, namespaceoid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c index 3e7b09b3494..bb2ce833bdd 100644 --- a/src/backend/commands/operatorcmds.c +++ b/src/backend/commands/operatorcmds.c @@ -33,6 +33,7 @@ #include "access/htup_details.h" #include "access/table.h" +#include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/objectaccess.h" #include "catalog/pg_namespace.h" @@ -92,6 +93,7 @@ DefineOperator(List *names, List *parameters) oprNamespace = QualifiedNameGetCreationNamespace(names, &oprName); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, oprNamespace); aclresult = object_aclcheck(NamespaceRelationId, oprNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -189,6 +191,7 @@ DefineOperator(List *names, List *parameters) if (typeName1) { + LockNotPinnedObjectById(TypeRelationId, typeId1); aclresult = object_aclcheck(TypeRelationId, typeId1, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, typeId1); @@ -196,6 +199,7 @@ DefineOperator(List *names, List *parameters) if (typeName2) { + LockNotPinnedObjectById(TypeRelationId, typeId2); aclresult = object_aclcheck(TypeRelationId, typeId2, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, typeId2); @@ -227,12 +231,14 @@ DefineOperator(List *names, List *parameters) * necessary, since EXECUTE will be checked at any attempted use of the * operator, but it seems like a good idea anyway. */ + LockNotPinnedObjectById(ProcedureRelationId, functionOid); aclresult = object_aclcheck(ProcedureRelationId, functionOid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, NameListToString(functionName)); rettype = get_func_rettype(functionOid); + LockNotPinnedObjectById(TypeRelationId, rettype); aclresult = object_aclcheck(TypeRelationId, rettype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, rettype); @@ -312,6 +318,7 @@ ValidateRestrictionEstimator(List *restrictionName) { AclResult aclresult; + LockNotPinnedObjectById(ProcedureRelationId, restrictionOid); aclresult = object_aclcheck(ProcedureRelationId, restrictionOid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) @@ -382,6 +389,7 @@ ValidateJoinEstimator(List *joinName) { AclResult aclresult; + LockNotPinnedObjectById(ProcedureRelationId, joinOid); aclresult = object_aclcheck(ProcedureRelationId, joinOid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index b354723be44..6afef0f55c4 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -185,6 +185,7 @@ CreateStatistics(CreateStatsStmt *stmt, bool check_rights) { AclResult aclresult; + LockNotPinnedObjectById(NamespaceRelationId, namespaceId); aclresult = object_aclcheck(NamespaceRelationId, namespaceId, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 523959ba0ce..08dea4b67b3 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -745,6 +745,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, conninfo = NULL; server = GetForeignServerByName(stmt->servername, false); + LockNotPinnedObjectById(ForeignServerRelationId, server->serverid); aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, owner, ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername); @@ -1833,6 +1834,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, * the server. */ new_server = GetForeignServerByName(stmt->servername, false); + LockNotPinnedObjectById(ForeignServerRelationId, new_server->serverid); aclresult = object_aclcheck(ForeignServerRelationId, new_server->serverid, form->subowner, ACL_USAGE); @@ -2262,6 +2264,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) ForeignServer *server; server = GetForeignServer(form->subserver); + LockNotPinnedObjectById(ForeignServerRelationId, form->subserver); aclresult = object_aclcheck(ForeignServerRelationId, form->subserver, form->subowner, ACL_USAGE); if (aclresult != ACLCHECK_OK) @@ -2606,6 +2609,7 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) { ForeignServer *server = GetForeignServer(form->subserver); + LockNotPinnedObjectById(ForeignServerRelationId, server->serverid); aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, newOwnerId, ACL_USAGE); if (aclresult != ACLCHECK_OK) ereport(ERROR, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 92b0f38c353..d95ef23a53f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -30,6 +30,7 @@ #include "access/xlog.h" #include "access/xloginsert.h" #include "catalog/catalog.h" +#include "catalog/dependency.h" #include "catalog/heap.h" #include "catalog/index.h" #include "catalog/namespace.h" @@ -997,6 +998,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, ofTypeId = typenameTypeId(NULL, stmt->ofTypename); + LockNotPinnedObjectById(TypeRelationId, ofTypeId); aclresult = object_aclcheck(TypeRelationId, ofTypeId, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, ofTypeId); @@ -1462,6 +1464,7 @@ BuildDescForRelation(const List *columns) attname = entry->colname; typenameTypeIdAndMod(NULL, entry->typeName, &atttypid, &atttypmod); + LockNotPinnedObjectById(TypeRelationId, atttypid); aclresult = object_aclcheck(TypeRelationId, atttypid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, atttypid); @@ -13941,6 +13944,7 @@ checkFkeyPermissions(Relation rel, int16 *attnums, int natts) int i; /* Okay if we have relation-level REFERENCES permission */ + LockNotPinnedObjectById(RelationRelationId, RelationGetRelid(rel)); aclresult = pg_class_aclcheck(RelationGetRelid(rel), roleid, ACL_REFERENCES); if (aclresult == ACLCHECK_OK) @@ -14724,6 +14728,7 @@ ATPrepAlterColumnType(List **wqueue, /* Look up the target type */ typenameTypeIdAndMod(pstate, typeName, &targettype, &targettypmod); + LockNotPinnedObjectById(TypeRelationId, targettype); aclresult = object_aclcheck(TypeRelationId, targettype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, targettype); @@ -16476,6 +16481,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock check_can_set_role(GetUserId(), newOwnerId); /* New owner must have CREATE privilege on namespace */ + LockNotPinnedObjectById(NamespaceRelationId, namespaceOid); aclresult = object_aclcheck(NamespaceRelationId, namespaceOid, newOwnerId, ACL_CREATE); if (aclresult != ACLCHECK_OK) @@ -19882,6 +19888,7 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid, */ if (IsA(stmt, RenameStmt)) { + LockNotPinnedObjectById(NamespaceRelationId, classform->relnamespace); aclresult = object_aclcheck(NamespaceRelationId, classform->relnamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index b87b4b40d07..3c5ca8fe7e3 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -350,6 +350,7 @@ CreateTriggerFiringOn(const CreateTrigStmt *stmt, const char *queryString, if (OidIsValid(constrrelid)) { + LockNotPinnedObjectById(RelationRelationId, constrrelid); aclresult = pg_class_aclcheck(constrrelid, GetUserId(), ACL_TRIGGER); if (aclresult != ACLCHECK_OK) @@ -695,6 +696,7 @@ CreateTriggerFiringOn(const CreateTrigStmt *stmt, const char *queryString, funcoid = LookupFuncName(stmt->funcname, 0, NULL, false); if (!isInternal) { + LockNotPinnedObjectById(ProcedureRelationId, funcoid); aclresult = object_aclcheck(ProcedureRelationId, funcoid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, diff --git a/src/backend/commands/tsearchcmds.c b/src/backend/commands/tsearchcmds.c index a12ce33bae4..eb9e68bbb91 100644 --- a/src/backend/commands/tsearchcmds.c +++ b/src/backend/commands/tsearchcmds.c @@ -414,6 +414,7 @@ DefineTSDictionary(List *names, List *parameters) namespaceoid = QualifiedNameGetCreationNamespace(names, &dictname); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, namespaceoid); aclresult = object_aclcheck(NamespaceRelationId, namespaceoid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -917,6 +918,7 @@ DefineTSConfiguration(List *names, List *parameters, ObjectAddress *copied) namespaceoid = QualifiedNameGetCreationNamespace(names, &cfgname); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, namespaceoid); aclresult = object_aclcheck(NamespaceRelationId, namespaceoid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index c4c3cdb5461..0e01dda90ba 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -39,6 +39,7 @@ #include "access/xact.h" #include "catalog/binary_upgrade.h" #include "catalog/catalog.h" +#include "catalog/dependency.h" #include "catalog/heap.h" #include "catalog/objectaccess.h" #include "catalog/pg_am.h" @@ -739,6 +740,7 @@ DefineDomain(ParseState *pstate, CreateDomainStmt *stmt) &domainName); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, domainNamespace); aclresult = object_aclcheck(NamespaceRelationId, domainNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) @@ -788,6 +790,7 @@ DefineDomain(ParseState *pstate, CreateDomainStmt *stmt) TypeNameToString(stmt->typeName)), parser_errposition(pstate, stmt->typeName->location))); + LockNotPinnedObjectById(TypeRelationId, basetypeoid); aclresult = object_aclcheck(TypeRelationId, basetypeoid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, basetypeoid); @@ -1195,6 +1198,7 @@ DefineEnum(CreateEnumStmt *stmt) &enumName); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, enumNamespace); aclresult = object_aclcheck(NamespaceRelationId, enumNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -1420,6 +1424,7 @@ DefineRange(ParseState *pstate, CreateRangeStmt *stmt) &typeName); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, typeNamespace); aclresult = object_aclcheck(NamespaceRelationId, typeNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_SCHEMA, @@ -1496,6 +1501,7 @@ DefineRange(ParseState *pstate, CreateRangeStmt *stmt) &multirangeTypeName); /* Check we have creation rights in target namespace */ + LockNotPinnedObjectById(NamespaceRelationId, multirangeNamespace); aclresult = object_aclcheck(NamespaceRelationId, multirangeNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) @@ -2421,6 +2427,7 @@ findRangeCanonicalFunction(List *procname, Oid typeOid) func_signature_string(procname, 1, NIL, argList)))); /* Also, range type's creator must have permission to call function */ + LockNotPinnedObjectById(ProcedureRelationId, procOid); aclresult = object_aclcheck(ProcedureRelationId, procOid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(procOid)); @@ -2464,6 +2471,7 @@ findRangeSubtypeDiffFunction(List *procname, Oid subtype) func_signature_string(procname, 2, NIL, argList)))); /* Also, range type's creator must have permission to call function */ + LockNotPinnedObjectById(ProcedureRelationId, procOid); aclresult = object_aclcheck(ProcedureRelationId, procOid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(procOid)); @@ -3963,6 +3971,7 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype) check_can_set_role(GetUserId(), newOwnerId); /* New owner must have CREATE privilege on namespace */ + LockNotPinnedObjectById(NamespaceRelationId, typTup->typnamespace); aclresult = object_aclcheck(NamespaceRelationId, typTup->typnamespace, newOwnerId, ACL_CREATE); diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h index fa508ea70c6..ac1d902e912 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -102,6 +102,7 @@ typedef struct ObjectAddresses ObjectAddresses; extern void AcquireDeletionLock(const ObjectAddress *object, int flags); extern void LockNotPinnedObject(const ObjectAddress *object); +extern void LockNotPinnedObjectById(Oid classid, Oid objid); extern bool isObjectPinned(const ObjectAddress *object); extern void ReleaseDeletionLock(const ObjectAddress *object); -- 2.34.1
>From 75143d7f2019248d1e403ae0a5045c89e85a06ec Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Mon, 27 Apr 2026 14:23:25 +0000 Subject: [PATCH v21 3/3] Add Assert guard to detect permission check before lock regressions Add instrumentation under USE_ASSERT_CHECKING to detect cases where object_aclcheck() is called on a referenced object before a lock is held on it, which would widen the TOCTOU window between the permission check and the dependency recording. In recordMultipleDependencies() and changeDependencyFor(), for each referenced object that had a permission check earlier in the same statement, assert that a lock is already held. This catches any future code that adds a permission check on a referenced object without first acquiring a lock. The tracking records each (classId, objectId, mode) from object_aclcheck() and pg_class_aclcheck(), filtering to only dependency-relevant ACL modes (ACL_CREATE, ACL_USAGE on non namespaces, ACL_EXECUTE, ACL_TRIGGER, ACL_REFERENCES). Tracking resets at each ProcessUtility() call. The tracking array is capped at 1024 entries per statement, which is sufficient for any practical DDL command. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] --- src/backend/catalog/aclchk.c | 32 ++++++++++++ src/backend/catalog/pg_depend.c | 52 ++++++++++++++++++++ src/backend/tcop/utility.c | 3 ++ src/include/catalog/aclcheck_track.h | 73 ++++++++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 161 insertions(+) 51.5% src/backend/catalog/ 46.7% src/include/catalog/ diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 007ede997c5..d13667f38a8 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -76,6 +76,7 @@ #include "parser/parse_type.h" #include "storage/lmgr.h" #include "utils/acl.h" +#include "catalog/aclcheck_track.h" #include "utils/aclchk_internal.h" #include "utils/builtins.h" #include "utils/fmgroids.h" @@ -3891,6 +3892,8 @@ object_aclcheck_ext(Oid classid, Oid objectid, Oid roleid, AclMode mode, bool *is_missing) { + aclcheck_track_record(classid, objectid, mode); + if (object_aclmask_ext(classid, objectid, roleid, mode, ACLMASK_ANY, is_missing) != 0) return ACLCHECK_OK; @@ -4093,6 +4096,8 @@ AclResult pg_class_aclcheck_ext(Oid table_oid, Oid roleid, AclMode mode, bool *is_missing) { + aclcheck_track_record(RelationRelationId, table_oid, mode); + if (pg_class_aclmask_ext(table_oid, roleid, mode, ACLMASK_ANY, is_missing) != 0) return ACLCHECK_OK; @@ -5036,3 +5041,30 @@ RemoveRoleFromInitPriv(Oid roleid, Oid classid, Oid objid, int32 objsubid) table_close(rel, RowExclusiveLock); } + +/* + * Instrumentation to detect permission check before lock regressions. + * Only used in assert-enabled builds. + */ +#ifdef USE_ASSERT_CHECKING +AclCheckEntry aclcheck_tracked[ACLCHECK_TRACK_MAX]; +int aclcheck_tracked_count = 0; + +void +aclcheck_track_reset(void) +{ + aclcheck_tracked_count = 0; +} + +bool +aclcheck_track_was_checked(Oid classId, Oid objectId) +{ + for (int i = 0; i < aclcheck_tracked_count; i++) + { + if (aclcheck_tracked[i].classId == classId && + aclcheck_tracked[i].objectId == objectId) + return true; + } + return false; +} +#endif /* USE_ASSERT_CHECKING */ diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 5618e3d26fa..28fa99b2d42 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -18,6 +18,7 @@ #include "access/htup_details.h" #include "access/table.h" #include "catalog/catalog.h" +#include "catalog/aclcheck_track.h" #include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/pg_constraint.h" @@ -27,6 +28,8 @@ #include "catalog/partition.h" #include "commands/extension.h" #include "miscadmin.h" +#include "storage/lmgr.h" +#include "storage/lock.h" #include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/rel.h" @@ -107,6 +110,31 @@ recordMultipleDependencies(const ObjectAddress *depender, if (isObjectPinned(referenced)) continue; +#ifdef USE_ASSERT_CHECKING + /* + * If this referenced object had a permission check earlier in this + * statement, assert that a lock is already held on it. This ensures + * callers acquired the lock before calling object_aclcheck(), not + * after. The latter would widen the TOCTOU window between the + * permission check and the dependency recording. + */ + if (aclcheck_track_was_checked(referenced->classId, referenced->objectId)) + { + if (referenced->classId == RelationRelationId) + Assert(CheckRelationOidLockedByMe(referenced->objectId, + AccessShareLock, true)); + else + { + LOCKTAG tag; + + SET_LOCKTAG_OBJECT(tag, MyDatabaseId, + referenced->classId, + referenced->objectId, 0); + Assert(LockHeldByMe(&tag, AccessShareLock, true)); + } + } +#endif + /* * Acquire a lock and check object still exists while recording the * dependency. @@ -511,6 +539,30 @@ changeDependencyFor(Oid classId, Oid objectId, return 1; } +#ifdef USE_ASSERT_CHECKING + /* + * If this referenced object had a permission check earlier in this + * statement, assert that a lock is already held on it. This ensures + * callers acquired the lock before calling object_aclcheck(), not after. + * The latter would widen the TOCTOU window between the permission check and + * the dependency recording. + */ + if (aclcheck_track_was_checked(objAddr.classId, objAddr.objectId)) + { + if (objAddr.classId == RelationRelationId) + Assert(CheckRelationOidLockedByMe(objAddr.objectId, + AccessShareLock, true)); + else + { + LOCKTAG tag; + + SET_LOCKTAG_OBJECT(tag, MyDatabaseId, + objAddr.classId, + objAddr.objectId, 0); + Assert(LockHeldByMe(&tag, AccessShareLock, true)); + } + } +#endif /* * Acquire a lock and check object still exists while changing the * dependency. diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 73a56f1df1d..0c4d382bd37 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -21,6 +21,7 @@ #include "access/xact.h" #include "access/xlog.h" #include "catalog/namespace.h" +#include "catalog/aclcheck_track.h" #include "catalog/pg_authid.h" #include "catalog/pg_inherits.h" #include "catalog/toasting.h" @@ -515,6 +516,8 @@ ProcessUtility(PlannedStmt *pstmt, Assert(queryString != NULL); /* required as of 8.4 */ Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN); + aclcheck_track_reset(); + /* * We provide a function hook variable that lets loadable plugins get * control when ProcessUtility is called. Such a plugin would normally diff --git a/src/include/catalog/aclcheck_track.h b/src/include/catalog/aclcheck_track.h new file mode 100644 index 00000000000..5825d7398d3 --- /dev/null +++ b/src/include/catalog/aclcheck_track.h @@ -0,0 +1,73 @@ +/*------------------------------------------------------------------------- + * + * aclcheck_track.h + * Instrumentation to detect permission check before lock via Assert in + * recordMultipleDependencies() and changeDependencyFor(). + * + * Only active in USE_ASSERT_CHECKING builds. + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/catalog/aclcheck_track.h + * + *------------------------------------------------------------------------- + */ +#ifndef ACLCHECK_TRACK_H +#define ACLCHECK_TRACK_H + +#ifdef USE_ASSERT_CHECKING + +#include "catalog/pg_namespace.h" + +#define ACLCHECK_TRACK_MAX 1024 + +typedef struct AclCheckEntry +{ + Oid classId; + Oid objectId; +} AclCheckEntry; + +extern AclCheckEntry aclcheck_tracked[]; +extern int aclcheck_tracked_count; + +extern void aclcheck_track_reset(void); +extern bool aclcheck_track_was_checked(Oid classId, Oid objectId); + +/* + * Only record aclchecks that are dependency-relevant: + * - ACL_CREATE on any object (creating something in a container) + * - ACL_USAGE on non-namespace objects (using a type, language, server) + * - ACL_EXECUTE on functions + * - ACL_TRIGGER, ACL_REFERENCES on relations + * + * Skip ACL_USAGE on namespaces — that's name resolution, not dependency. + */ +static inline void +aclcheck_track_record(Oid classId, Oid objectId, AclMode mode) +{ + /* Skip ACL_USAGE on namespaces (name resolution, not dependency) */ + if (classId == NamespaceRelationId && !(mode & ACL_CREATE)) + return; + + /* Only track dependency-relevant modes */ + if (!(mode & (ACL_CREATE | ACL_USAGE | ACL_EXECUTE | ACL_TRIGGER | ACL_REFERENCES))) + return; + + if (aclcheck_tracked_count < ACLCHECK_TRACK_MAX) + { + aclcheck_tracked[aclcheck_tracked_count].classId = classId; + aclcheck_tracked[aclcheck_tracked_count].objectId = objectId; + aclcheck_tracked_count++; + } +} + +#else /* !USE_ASSERT_CHECKING */ + +#define aclcheck_track_reset() ((void) 0) +#define aclcheck_track_record(c, o, m) ((void) 0) +#define aclcheck_track_was_checked(c, o) (false) + +#endif /* USE_ASSERT_CHECKING */ + +#endif /* ACLCHECK_TRACK_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 8cf40c87043..cb0cc25bda8 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -19,6 +19,7 @@ AbsoluteTime AccessMethodInfo AccessPriv Acl +AclCheckEntry AclItem AclMaskHow AclMode -- 2.34.1
