Subject: [PATCH v3] Tighten ACL check in repack_is_permitted_for_relation()

From: Álvaro Herrera <[email-masked]>
Date: Mon, 17 Aug 2026 17:53:34 +0200

repack_is_permitted_for_relation() uses pg_class_aclcheck_ext()
to silently skip a concurrently-dropped relation.  That's wrong
for a caller that may already hold a lock on the relation whose
ACL is checked, where missing a relation is not fine, and it
makes the single-relation REPACK and CLUSTER cases more brittle.
So only detect a missing relation where that's expected,
following the fix for vacuum_is_permitted_for_relation() in
commit 824d5f6241ea.

The new already_locked behavior is limited to get_tables_to_repack()
and get_tables_to_repack_partitioned().  All other callers of
repack_is_permitted_for_relation() hold a lock on the relation
that prevents it from being concurrently dropped, so this commit
also adds an assertion to that effect.

Author: Bharath Rupireddy <[email-masked]>
Discussion: https://www.postgresql.org/message-id/CALj2ACX3pyuRS8%2B%2B6L20cJUMRTf_qbbVp69J1btJ3y6%3D77e5gw%40mail.gmail.com
---
 src/backend/commands/repack.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index edff54e734e..66b88e28c2e 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -173,7 +173,8 @@ static List *get_tables_to_repack_partitioned(RepackStmt *stmt,
 											  Relation rel,
 											  MemoryContext permcxt);
 static bool repack_is_permitted_for_relation(RepackCommand cmd,
-											 Oid relid, Oid userid);
+											 Oid relid, Oid userid,
+											 bool already_locked);
 
 static void apply_concurrent_changes(BufFile *file, ChangeContext *chgcxt);
 static void apply_concurrent_insert(Relation rel, TupleTableSlot *slot,
@@ -681,7 +682,7 @@ cluster_rel_recheck(RepackCommand cmd, Relation OldHeap, Oid indexOid,
 	Assert(CheckRelationLockedByMe(OldHeap, lmode, false));
 
 	/* Check that the user still has privileges for the relation */
-	if (!repack_is_permitted_for_relation(cmd, tableOid, userid))
+	if (!repack_is_permitted_for_relation(cmd, tableOid, userid, true))
 	{
 		relation_close(OldHeap, lmode);
 		return false;
@@ -2155,7 +2156,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
-												  GetUserId()))
+												  GetUserId(), false))
 				continue;
 
 			/* Use a permanent memory context for the result list */
@@ -2192,7 +2193,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
-												  GetUserId()))
+												  GetUserId(), false))
 				continue;
 
 			/* Use a permanent memory context for the result list */
@@ -2321,7 +2322,7 @@ get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel,
 		 * if so.
 		 */
 		if (!repack_is_permitted_for_relation(stmt->command, table_oid,
-											  GetUserId()))
+											  GetUserId(), false))
 			continue;
 
 		/* Use a permanent memory context for the result list */
@@ -2341,26 +2342,38 @@ get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel,

 /*
- * Return whether userid has privileges to execute REPACK on relid.
+ * Return whether userid has privileges to execute REPACK/CLUSTER on relid.
  *
- * Caller may not have a lock on the relation, so it could have been
- * dropped concurrently.  In that case, silently return false.
+ * The relation may already be locked by caller, in which case it cannot
+ * possibly go missing; otherwise it can have been removed recently.  If
+ * it's been removed, silently return false.  If the relation does exist but
+ * the user doesn't have the required privs, emit a WARNING and return false.
  *
- * If the relation does exist but the user doesn't have the required
- * privs, emit a WARNING and return false.  Otherwise, return true.
+ * Otherwise, return true.
  */
 static bool
-repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid)
+repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid,
+								 bool already_locked)
 {
 	bool		is_missing = false;
 	AclResult	result;
 	char	   *relname;
 
 	Assert(cmd == REPACK_COMMAND_CLUSTER || cmd == REPACK_COMMAND_REPACK);
+	Assert(!already_locked ||
+		   CheckRelationOidLockedByMe(relid, AccessShareLock, true));
 
 	result = pg_class_aclcheck_ext(relid, userid, ACL_MAINTAIN, &is_missing);
+
+	/*
+	 * If the relation was concurrently dropped, nothing to do.  This is only
+	 * reachable when the caller doesn't already have a lock on the relation.
+	 */
 	if (is_missing)
+	{
+		Assert(!already_locked);
 		return false;
+	}
 
 	if (result == ACLCHECK_OK)
 		return true;
-- 
2.47.3

--lagmlhwoe2cxax2o--