mirror of
https://github.com/postgres/postgres.git
synced 2025-07-09 22:41:56 +03:00
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 by19de0ab23c
. To fix this problem, this commit partially revertsff9618e82a
. 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
This commit is contained in:
@ -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,
|
||||
|
@ -2853,11 +2853,14 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
|
||||
|
||||
/* Check permissions */
|
||||
table_oid = IndexGetRelation(relId, true);
|
||||
if (OidIsValid(table_oid) &&
|
||||
pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
|
||||
!has_partition_ancestor_privs(table_oid, GetUserId(), ACL_MAINTAIN))
|
||||
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX,
|
||||
relation->relname);
|
||||
if (OidIsValid(table_oid))
|
||||
{
|
||||
AclResult aclresult;
|
||||
|
||||
aclresult = pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN);
|
||||
if (aclresult != ACLCHECK_OK)
|
||||
aclcheck_error(aclresult, OBJECT_INDEX, relation->relname);
|
||||
}
|
||||
|
||||
/* Lock heap before index to avoid deadlock. */
|
||||
if (relId != oldRelId)
|
||||
@ -3064,18 +3067,12 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
|
||||
continue;
|
||||
|
||||
/*
|
||||
* The table can be reindexed if the user has been granted MAINTAIN on
|
||||
* the table or one of its partition ancestors or the user is a
|
||||
* superuser, the table owner, or the database/schema owner (but in
|
||||
* the latter case, only if it's not a shared relation).
|
||||
* pg_class_aclcheck includes the superuser case, and depending on
|
||||
* objectKind we already know that the user has permission to run
|
||||
* REINDEX on this database or schema per the permission checks at the
|
||||
* beginning of this routine.
|
||||
* We already checked privileges on the database or schema, but we
|
||||
* further restrict reindexing shared catalogs to roles with the
|
||||
* MAINTAIN privilege on the relation.
|
||||
*/
|
||||
if (classtuple->relisshared &&
|
||||
pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
|
||||
!has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
|
||||
pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
|
||||
continue;
|
||||
|
||||
/*
|
||||
|
@ -19,7 +19,6 @@
|
||||
#include "catalog/namespace.h"
|
||||
#include "catalog/pg_inherits.h"
|
||||
#include "commands/lockcmds.h"
|
||||
#include "commands/tablecmds.h"
|
||||
#include "miscadmin.h"
|
||||
#include "nodes/nodeFuncs.h"
|
||||
#include "parser/parse_clause.h"
|
||||
@ -297,12 +296,5 @@ LockTableAclCheck(Oid reloid, LOCKMODE lockmode, Oid userid)
|
||||
|
||||
aclresult = pg_class_aclcheck(reloid, userid, aclmask);
|
||||
|
||||
/*
|
||||
* If this is a partition, check permissions of its ancestors if needed.
|
||||
*/
|
||||
if (aclresult != ACLCHECK_OK &&
|
||||
has_partition_ancestor_privs(reloid, userid, ACL_MAINTAIN))
|
||||
aclresult = ACLCHECK_OK;
|
||||
|
||||
return aclresult;
|
||||
}
|
||||
|
@ -16986,6 +16986,7 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
|
||||
Oid relId, Oid oldRelId, void *arg)
|
||||
{
|
||||
char relkind;
|
||||
AclResult aclresult;
|
||||
|
||||
/* Nothing to do if the relation was not found. */
|
||||
if (!OidIsValid(relId))
|
||||
@ -17006,36 +17007,9 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
|
||||
errmsg("\"%s\" is not a table or materialized view", relation->relname)));
|
||||
|
||||
/* Check permissions */
|
||||
if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
|
||||
!has_partition_ancestor_privs(relId, GetUserId(), ACL_MAINTAIN))
|
||||
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE,
|
||||
relation->relname);
|
||||
}
|
||||
|
||||
/*
|
||||
* If relid is a partition, returns whether userid has any of the privileges
|
||||
* specified in acl on any of its ancestors. Otherwise, returns false.
|
||||
*/
|
||||
bool
|
||||
has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl)
|
||||
{
|
||||
List *ancestors;
|
||||
ListCell *lc;
|
||||
|
||||
if (!get_rel_relispartition(relid))
|
||||
return false;
|
||||
|
||||
ancestors = get_partition_ancestors(relid);
|
||||
foreach(lc, ancestors)
|
||||
{
|
||||
Oid ancestor = lfirst_oid(lc);
|
||||
|
||||
if (OidIsValid(ancestor) &&
|
||||
pg_class_aclcheck(ancestor, userid, acl) == ACLCHECK_OK)
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
aclresult = pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN);
|
||||
if (aclresult != ACLCHECK_OK)
|
||||
aclcheck_error(aclresult, OBJECT_TABLE, relation->relname);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -41,7 +41,6 @@
|
||||
#include "catalog/pg_namespace.h"
|
||||
#include "commands/cluster.h"
|
||||
#include "commands/defrem.h"
|
||||
#include "commands/tablecmds.h"
|
||||
#include "commands/vacuum.h"
|
||||
#include "miscadmin.h"
|
||||
#include "nodes/makefuncs.h"
|
||||
@ -721,17 +720,12 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
|
||||
/*----------
|
||||
* A role has privileges to vacuum or analyze the relation if any of the
|
||||
* following are true:
|
||||
* - the role is a superuser
|
||||
* - the role owns the relation
|
||||
* - the role owns the current database and the relation is not shared
|
||||
* - the role has been granted the MAINTAIN privilege on the relation
|
||||
* - the role has privileges to vacuum/analyze any of the relation's
|
||||
* partition ancestors
|
||||
* - the role has the MAINTAIN privilege on the relation
|
||||
*----------
|
||||
*/
|
||||
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) ||
|
||||
pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK ||
|
||||
has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
|
||||
pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
|
||||
return true;
|
||||
|
||||
relname = NameStr(reltuple->relname);
|
||||
|
Reference in New Issue
Block a user