postgres.git / summary / log / commit / refs
commit 941e0c0df2bcdeb63bd822a1c46818fa9d7d8599
Author: Noah Misch <noah@leadboat.com>
Date: Mon Nov 25 22:42:35 2024 +0000
First released in: 18.0, 13.19, 14.16, 15.11, 16.7, 17.3
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;
[parent: 851c6ff18fd3]