From e8c78edb6708f5d320633c95b342f9afd4b086b5 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 25 Nov 2024 14:42:35 -0800 Subject: [PATCH] Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE. 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 --- src/backend/utils/cache/syscache.c | 15 ++++++++++----- src/test/regress/expected/tablespace.out | 5 +++++ src/test/regress/sql/tablespace.sql | 6 ++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c index 2f619f61807..e8349a42966 100644 --- a/src/backend/utils/cache/syscache.c +++ b/src/backend/utils/cache/syscache.c @@ -1242,11 +1242,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 @@ -1298,8 +1296,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 index c52cf1cfcf9..3873e3cd7fc 100644 --- a/src/test/regress/expected/tablespace.out +++ b/src/test/regress/expected/tablespace.out @@ -929,6 +929,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 index 21db433f2a8..060698072e6 100644 --- a/src/test/regress/sql/tablespace.sql +++ b/src/test/regress/sql/tablespace.sql @@ -398,6 +398,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;