From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Subject: [PATCH v21 3/3] Add Assert guard to detect permission check before lock regressions
Date: Mon, 27 Apr 2026 14:23:25 +0000
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 <bertranddrouvot.pg@gmail.com>
Reviewed-by:
Discussion: https://postgr.es/m/ZiYjn0eVc7pxVY45@ip-10-97-1-34.eu-west-3.compute.internal
---
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.cindex 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.cindex 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.cindex 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.hnew file mode 100644index 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.listindex 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
--FjYZKkuBdH+F20sI--
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com
Subject: Re: [PATCH v21 3/3] Add Assert guard to detect permission check before lock regressions
In-Reply-To: <no-message-id-94773@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox