From 4dbdb82513b65ab3a29e431f0233009c8a0fa890 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Thu, 22 Jun 2023 15:48:20 -0700 Subject: Fix cache lookup hazards introduced by ff9618e82a. ff9618e82a introduced has_partition_ancestor_privs(), which is used to check whether a user has MAINTAIN on any partition ancestors. This involves syscache lookups, and presently this function does not take any relation locks, so it is likely subject to the same kind of cache lookup failures that were fixed by 19de0ab23c. To fix this problem, this commit partially reverts ff9618e82a. Specifically, it removes the partition-related changes, including the has_partition_ancestor_privs() function mentioned above. This means that MAINTAIN on a partitioned table is no longer sufficient to perform maintenance commands on its partitions. This is more like how privileges for maintenance commands work on supported versions. Privileges are checked for each partition, so a command that flows down to all partitions might refuse to process them (e.g., if the current user doesn't have MAINTAIN on the partition). In passing, adjust a few related comments and error messages, and add a test for the privilege checks for CLUSTER on a partitioned table. Reviewed-by: Michael Paquier, Jeff Davis Discussion: https://postgr.es/m/20230613211246.GA219055%40nathanxps13 --- src/backend/commands/cluster.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'src/backend/commands/cluster.c') diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 369fea7c046..3bfabb6d10b 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1694,10 +1694,13 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) continue; /* - * We already checked that the user has privileges to CLUSTER the - * partitioned table when we locked it earlier, so there's no need to - * check the privileges again here. + * It's possible that the user does not have privileges to CLUSTER the + * leaf partition despite having such privileges on the partitioned + * table. We skip any partitions which the user is not permitted to + * CLUSTER. */ + if (!cluster_is_permitted_for_relation(relid, GetUserId())) + continue; /* Use a permanent memory context for the result list */ old_context = MemoryContextSwitchTo(cluster_context); @@ -1720,8 +1723,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) static bool cluster_is_permitted_for_relation(Oid relid, Oid userid) { - if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK || - has_partition_ancestor_privs(relid, userid, ACL_MAINTAIN)) + if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK) return true; ereport(WARNING, -- cgit v1.2.3