Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE.

nmisch · nmisch · commit 718af10dab44 · 2024-11-25T14:42:38.000-08:00
This WARNING appeared because SearchSysCacheLocked1() read cc_relisshared before catcache initialization, when the field is false unconditionally. On the basis of reading false there, it constructed a locktag as though pg_tablespace weren't relisshared. Only shared catalogs could be affected, and only GRANT TABLESPACE was affected in practice. SearchSysCacheLocked1() callers use one other shared-relation syscache, DATABASEOID. DATABASEOID is initialized by the end of CheckMyDatabase(), making the problem unreachable for pg_database. Back-patch to v13 (all supported versions). This has no known impact before v16, where ExecGrant_common() first appeared. Earlier branches avoid trouble by having a separate ExecGrant_Tablespace() that doesn't use LOCKTAG_TUPLE. However, leaving this unfixed in v15 could ensnare a future back-patch of a SearchSysCacheLocked1() call. Reported by Aya Iwata. Discussion: https://postgr.es/m/OS7PR01MB11964507B5548245A7EE54E70EA212@OS7PR01MB11964.jpnprd01.prod.outlook.com
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
@@ -287,11 +287,9 @@ HeapTuple
 SearchSysCacheLocked1(int cacheId,
 					  Datum key1)
 {
+	CatCache   *cache = SysCache[cacheId];
 	ItemPointerData tid;
 	LOCKTAG		tag;
-	Oid			dboid =
-		SysCache[cacheId]->cc_relisshared ? InvalidOid : MyDatabaseId;
-	Oid			reloid = cacheinfo[cacheId].reloid;
 
 	/*----------
 	 * Since inplace updates may happen just before our LockTuple(), we must
@@ -343,8 +341,15 @@ SearchSysCacheLocked1(int cacheId,
 
 		tid = tuple->t_self;
 		ReleaseSysCache(tuple);
-		/* like: LockTuple(rel, &tid, lockmode) */
-		SET_LOCKTAG_TUPLE(tag, dboid, reloid,
+
+		/*
+		 * Do like LockTuple(rel, &tid, lockmode).  While cc_relisshared won't
+		 * change from one iteration to another, it may have been a temporary
+		 * "false" until our first SearchSysCache1().
+		 */
+		SET_LOCKTAG_TUPLE(tag,
+						  cache->cc_relisshared ? InvalidOid : MyDatabaseId,
+						  cache->cc_reloid,
 						  ItemPointerGetBlockNumber(&tid),
 						  ItemPointerGetOffsetNumber(&tid));
 		(void) LockAcquire(&tag, lockmode, false, false);
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
@@ -927,6 +927,11 @@ ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
 -- Fail, not empty
 DROP TABLESPACE regress_tblspace;
 ERROR:  tablespace "regress_tblspace" is not empty
+-- Adequate cache initialization before GRANT
+\c -
+BEGIN;
+GRANT ALL ON TABLESPACE regress_tblspace TO PUBLIC;
+ROLLBACK;
 CREATE ROLE regress_tablespace_user1 login;
 CREATE ROLE regress_tablespace_user2 login;
 GRANT USAGE ON SCHEMA testschema TO regress_tablespace_user2;
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
@@ -396,6 +396,12 @@ ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
 -- Fail, not empty
 DROP TABLESPACE regress_tblspace;
 
+-- Adequate cache initialization before GRANT
+\c -
+BEGIN;
+GRANT ALL ON TABLESPACE regress_tblspace TO PUBLIC;
+ROLLBACK;
+
 CREATE ROLE regress_tablespace_user1 login;
 CREATE ROLE regress_tablespace_user2 login;
 GRANT USAGE ON SCHEMA testschema TO regress_tablespace_user2;
