public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bertrand Drouvot <[email protected]>
Subject: [PATCH v20 2/3] Lock referenced objects before permission checks in DDL commands
Date: Mon, 27 Apr 2026 14:19:51 +0000
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 | 8 ++++++++
src/include/catalog/dependency.h | 1 +
22 files changed, 95 insertions(+)
22.8% src/backend/catalog/
75.8% 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 1e10d9d9a58..560f8bff629 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);
@@ -2253,6 +2255,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)
@@ -2597,6 +2600,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 d8d7969bf30..decd5071ea6 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 da0d1ba6791..dab6067bbf9 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 cd38e9cddf4..d5e76fbb241 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,
@@ -2414,6 +2419,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));
@@ -2457,6 +2463,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));
@@ -3956,6 +3963,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
--zB/Wek73NRZcta1g
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v20-0003-Add-Assert-guard-to-detect-permission-check-befo.patch"
Content-Transfer-Encoding: 8bit
view thread (1000+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v20 2/3] Lock referenced objects before permission checks in DDL commands
In-Reply-To: <no-message-id-355720@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox