diff options
author | Heikki Linnakangas <heikki.linnakangas@iki.fi> | 2021-02-08 11:01:51 +0200 |
---|---|---|
committer | Heikki Linnakangas <heikki.linnakangas@iki.fi> | 2021-02-08 11:01:55 +0200 |
commit | cb5868cc1bd77b9b2f0f62b28d15a62b97ba3e94 (patch) | |
tree | e5de42569e8bf3561d0ce0c2112d7292478db9aa /src/backend/executor | |
parent | 1c91af9b26a0252f43dc581cb0e607206fbebe4a (diff) | |
download | postgresql-cb5868cc1bd77b9b2f0f62b28d15a62b97ba3e94.tar.gz postgresql-cb5868cc1bd77b9b2f0f62b28d15a62b97ba3e94.zip |
Fix permission checks on constraint violation errors on partitions.
If a cross-partition UPDATE violates a constraint on the target partition,
and the columns in the new partition are in different physical order than
in the parent, the error message can reveal columns that the user does not
have SELECT permission on. A similar bug was fixed earlier in commit
804b6b6db4.
The cause of the bug is that the callers of the
ExecBuildSlotValueDescription() function got confused when constructing
the list of modified columns. If the tuple was routed from a parent, we
converted the tuple to the parent's format, but the list of modified
columns was grabbed directly from the child's RTE entry.
ExecUpdateLockMode() had a similar issue. That lead to confusion on which
columns are key columns, leading to wrong tuple lock being taken on tables
referenced by foreign keys, when a row is updated with INSERT ON CONFLICT
UPDATE. A new isolation test is added for that corner case.
With this patch, the ri_RangeTableIndex field is no longer set for
partitions that don't have an entry in the range table. Previously, it was
set to the RTE entry of the parent relation, but that was confusing.
NOTE: This modifies the ResultRelInfo struct, replacing the
ri_PartitionRoot field with ri_RootResultRelInfo. That's a bit risky to
backpatch, because it breaks any extensions accessing the field. The
change that ri_RangeTableIndex is not set for partitions could potentially
break extensions, too. The ResultRelInfos are visible to FDWs at least,
and this patch required small changes to postgres_fdw. Nevertheless, this
seem like the least bad option. I don't think these fields widely used in
extensions; I don't think there are FDWs out there that uses the FDW
"direct update" API, other than postgres_fdw. If there is, you will get a
compilation error, so hopefully it is caught quickly.
Backpatch to 11, where support for both cross-partition UPDATEs, and unique
indexes on partitioned tables, were added.
Reviewed-by: Amit Langote
Security: CVE-2021-3393
Diffstat (limited to 'src/backend/executor')
-rw-r--r-- | src/backend/executor/execMain.c | 97 | ||||
-rw-r--r-- | src/backend/executor/execPartition.c | 36 | ||||
-rw-r--r-- | src/backend/executor/execUtils.c | 64 | ||||
-rw-r--r-- | src/backend/executor/nodeModifyTable.c | 7 |
4 files changed, 134 insertions, 70 deletions
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 8daf0fe9330..f852ac4faa9 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -102,17 +102,6 @@ static char *ExecBuildSlotValueDescription(Oid reloid, static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree); -/* - * Note that GetUpdatedColumns() also exists in commands/trigger.c. There does - * not appear to be any good header to put it into, given the structures that - * it uses, so we let them be duplicated. Be sure to update both if one needs - * to be changed, however. - */ -#define GetInsertedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->insertedCols) -#define GetUpdatedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols) - /* end of local decls */ @@ -1302,7 +1291,7 @@ void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, - Relation partition_root, + ResultRelInfo *partition_root_rri, int instrument_options) { List *partition_check = NIL; @@ -1363,7 +1352,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, partition_check = RelationGetPartitionQual(resultRelationDesc); resultRelInfo->ri_PartitionCheck = partition_check; - resultRelInfo->ri_PartitionRoot = partition_root; + resultRelInfo->ri_RootResultRelInfo = partition_root_rri; resultRelInfo->ri_PartitionReadyForRouting = false; } @@ -1906,26 +1895,28 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate) { + Oid root_relid; Relation rel = resultRelInfo->ri_RelationDesc; Relation orig_rel = rel; TupleDesc tupdesc = RelationGetDescr(rel); char *val_desc; Bitmapset *modifiedCols; - Bitmapset *insertedCols; - Bitmapset *updatedCols; /* * Need to first convert the tuple to the root partitioned table's row * type. For details, check similar comments in ExecConstraints(). */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); - TupleDesc old_tupdesc = RelationGetDescr(rel); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; + TupleDesc old_tupdesc; TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + root_relid = RelationGetRelid(rootrel->ri_RelationDesc); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); + + old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -1936,12 +1927,18 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + } + else + { + root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc); + tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc); + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); } - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); - val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), + val_desc = ExecBuildSlotValueDescription(root_relid, slot, tupdesc, modifiedCols, @@ -1972,8 +1969,6 @@ ExecConstraints(ResultRelInfo *resultRelInfo, TupleDesc tupdesc = RelationGetDescr(rel); TupleConstr *constr = tupdesc->constr; Bitmapset *modifiedCols; - Bitmapset *insertedCols; - Bitmapset *updatedCols; Assert(constr || resultRelInfo->ri_PartitionCheck); @@ -1999,13 +1994,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo, * rowtype so that val_desc shown error message matches the * input tuple. */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(orig_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -2016,11 +2011,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + rel = rootrel->ri_RelationDesc; } - - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); + else + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -2047,14 +2044,14 @@ ExecConstraints(ResultRelInfo *resultRelInfo, Relation orig_rel = rel; /* See the comment above. */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; TupleDesc old_tupdesc = RelationGetDescr(rel); TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -2065,11 +2062,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + rel = rootrel->ri_RelationDesc; } - - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); + else + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -2138,8 +2137,6 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, { char *val_desc; Bitmapset *modifiedCols; - Bitmapset *insertedCols; - Bitmapset *updatedCols; switch (wco->kind) { @@ -2154,14 +2151,14 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, */ case WCO_VIEW_CHECK: /* See the comment in ExecConstraints(). */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; TupleDesc old_tupdesc = RelationGetDescr(rel); TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -2172,11 +2169,13 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + rel = rootrel->ri_RelationDesc; } - - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); + else + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -2390,7 +2389,7 @@ ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo) * been modified, then we can use a weaker lock, allowing for better * concurrency. */ - updatedCols = GetUpdatedColumns(relinfo, estate); + updatedCols = ExecGetUpdatedCols(relinfo, estate); keyCols = RelationGetIndexAttrBitmap(relinfo->ri_RelationDesc, INDEX_ATTR_BITMAP_KEY); diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index dc58fd0a82c..40b5d3717fd 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -107,7 +107,7 @@ static void find_matching_subplans_recurse(PartitionPruningData *prunedata, * the partitions to route tuples to. See ExecPrepareTupleRouting. */ PartitionTupleRouting * -ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) +ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, ResultRelInfo *rootResultRelInfo) { List *leaf_parts; ListCell *cell; @@ -123,10 +123,12 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) * Get the information about the partition tree after locking all the * partitions. */ - (void) find_all_inheritors(RelationGetRelid(rel), RowExclusiveLock, NULL); + (void) find_all_inheritors(RelationGetRelid(rootResultRelInfo->ri_RelationDesc), + RowExclusiveLock, NULL); proute = (PartitionTupleRouting *) palloc0(sizeof(PartitionTupleRouting)); proute->partition_dispatch_info = - RelationGetPartitionDispatchInfo(rel, &proute->num_dispatch, + RelationGetPartitionDispatchInfo(rootResultRelInfo->ri_RelationDesc, + &proute->num_dispatch, &leaf_parts); proute->num_partitions = nparts = list_length(leaf_parts); proute->partitions = @@ -186,7 +188,7 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) * descriptor. When generating the per-subplan result rels, this * was not set. */ - leaf_part_rri->ri_PartitionRoot = rel; + leaf_part_rri->ri_RootResultRelInfo = rootResultRelInfo; /* Remember the subplan offset for this ResultRelInfo */ proute->subplan_partition_offsets[update_rri_index] = i; @@ -366,13 +368,13 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, */ ResultRelInfo * ExecInitPartitionInfo(ModifyTableState *mtstate, - ResultRelInfo *resultRelInfo, + ResultRelInfo *rootResultRelInfo, PartitionTupleRouting *proute, EState *estate, int partidx) { ModifyTable *node = (ModifyTable *) mtstate->ps.plan; - Relation rootrel = resultRelInfo->ri_RelationDesc, - partrel; + Relation partrel; + int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; Relation firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc; ResultRelInfo *leaf_part_rri; MemoryContext oldContext; @@ -394,8 +396,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, leaf_part_rri = makeNode(ResultRelInfo); InitResultRelInfo(leaf_part_rri, partrel, - node ? node->nominalRelation : 1, - rootrel, + 0, + rootResultRelInfo, estate->es_instrument); /* @@ -441,7 +443,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, List *wcoList; List *wcoExprs = NIL; ListCell *ll; - int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; /* * In the case of INSERT on a partitioned table, there is only one @@ -507,7 +508,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, TupleTableSlot *slot; ExprContext *econtext; List *returningList; - int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; /* See the comment above for WCO lists. */ Assert((node->operation == CMD_INSERT && @@ -568,7 +568,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, if (node && node->onConflictAction != ONCONFLICT_NONE) { TupleConversionMap *map = proute->parent_child_tupconv_maps[partidx]; - int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; TupleDesc partrelDesc = RelationGetDescr(partrel); ExprContext *econtext = mtstate->ps.ps_ExprContext; ListCell *lc; @@ -580,7 +579,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * list and searching for ancestry relationships to each index in the * ancestor table. */ - if (list_length(resultRelInfo->ri_onConflictArbiterIndexes) > 0) + if (list_length(rootResultRelInfo->ri_onConflictArbiterIndexes) > 0) { List *childIdxs; @@ -593,7 +592,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, ListCell *lc2; ancestors = get_partition_ancestors(childIdx); - foreach(lc2, resultRelInfo->ri_onConflictArbiterIndexes) + foreach(lc2, rootResultRelInfo->ri_onConflictArbiterIndexes) { if (list_member_oid(ancestors, lfirst_oid(lc2))) arbiterIndexes = lappend_oid(arbiterIndexes, childIdx); @@ -607,7 +606,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * (This shouldn't happen, since arbiter index selection should not * pick up an invalid index.) */ - if (list_length(resultRelInfo->ri_onConflictArbiterIndexes) != + if (list_length(rootResultRelInfo->ri_onConflictArbiterIndexes) != list_length(arbiterIndexes)) elog(ERROR, "invalid arbiter index list"); leaf_part_rri->ri_onConflictArbiterIndexes = arbiterIndexes; @@ -618,7 +617,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, if (node->onConflictAction == ONCONFLICT_UPDATE) { Assert(node->onConflictSet != NIL); - Assert(resultRelInfo->ri_onConflict != NULL); + Assert(rootResultRelInfo->ri_onConflict != NULL); /* * If the partition's tuple descriptor matches exactly the root @@ -627,7 +626,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * need to create state specific to this partition. */ if (map == NULL) - leaf_part_rri->ri_onConflict = resultRelInfo->ri_onConflict; + leaf_part_rri->ri_onConflict = rootResultRelInfo->ri_onConflict; else { List *onconflset; @@ -737,6 +736,7 @@ ExecInitRoutingInfo(ModifyTableState *mtstate, ResultRelInfo *partRelInfo, int partidx) { + ResultRelInfo *rootRelInfo = partRelInfo->ri_RootResultRelInfo; MemoryContext oldContext; /* @@ -749,7 +749,7 @@ ExecInitRoutingInfo(ModifyTableState *mtstate, * partition from the parent's type to the partition's. */ proute->parent_child_tupconv_maps[partidx] = - convert_tuples_by_name(RelationGetDescr(partRelInfo->ri_PartitionRoot), + convert_tuples_by_name(RelationGetDescr(rootRelInfo->ri_RelationDesc), RelationGetDescr(partRelInfo->ri_RelationDesc), gettext_noop("could not convert row type")); diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 5b3eaec80bc..e91a6ebddd5 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -45,6 +45,7 @@ #include "access/relscan.h" #include "access/transam.h" #include "executor/executor.h" +#include "executor/execPartition.h" #include "jit/jit.h" #include "mb/pg_wchar.h" #include "nodes/nodeFuncs.h" @@ -1068,3 +1069,66 @@ ExecCleanTargetListLength(List *targetlist) } return len; } + +/* Return a bitmap representing columns being inserted */ +Bitmapset * +ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate) +{ + /* + * The columns are stored in the range table entry. If this ResultRelInfo + * doesn't have an entry in the range table (i.e. if it represents a + * partition routing target), fetch the parent's RTE and map the columns + * to the order they are in the partition. + */ + if (relinfo->ri_RangeTableIndex != 0) + { + RangeTblEntry *rte = rt_fetch(relinfo->ri_RangeTableIndex, + estate->es_range_table); + + return rte->insertedCols; + } + else + { + ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo; + RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex, + estate->es_range_table); + TupleConversionMap *map; + + map = convert_tuples_by_name(RelationGetDescr(rootRelInfo->ri_RelationDesc), + RelationGetDescr(relinfo->ri_RelationDesc), + gettext_noop("could not convert row type")); + if (map != NULL) + return execute_attr_map_cols(rte->insertedCols, map); + else + return rte->insertedCols; + } +} + +/* Return a bitmap representing columns being updated */ +Bitmapset * +ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate) +{ + /* see ExecGetInsertedCols() */ + if (relinfo->ri_RangeTableIndex != 0) + { + RangeTblEntry *rte = rt_fetch(relinfo->ri_RangeTableIndex, + estate->es_range_table); + + return rte->updatedCols; + } + else + { + ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo; + RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex, + estate->es_range_table); + TupleConversionMap *map; + + map = convert_tuples_by_name(RelationGetDescr(rootRelInfo->ri_RelationDesc), + RelationGetDescr(relinfo->ri_RelationDesc), + gettext_noop("could not convert row type")); + if (map != NULL) + return execute_attr_map_cols(rte->updatedCols, map); + else + return rte->updatedCols; + } +} diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index c84171973c9..a0fd6c15d29 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -403,7 +403,7 @@ ExecInsert(ModifyTableState *mtstate, * if there's no BR trigger defined on the partition. */ if (resultRelInfo->ri_PartitionCheck && - (resultRelInfo->ri_PartitionRoot == NULL || + (resultRelInfo->ri_RootResultRelInfo == NULL || (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_insert_before_row))) ExecPartitionCheck(resultRelInfo, slot, estate, true); @@ -2324,7 +2324,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) estate->es_result_relation_info = saved_resultRelInfo; /* Get the target relation */ - rel = (getTargetResultRelInfo(mtstate))->ri_RelationDesc; + resultRelInfo = getTargetResultRelInfo(mtstate); + rel = resultRelInfo->ri_RelationDesc; /* * If it's not a partitioned table after all, UPDATE tuple routing should @@ -2340,7 +2341,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && (operation == CMD_INSERT || update_tuple_routing_needed)) mtstate->mt_partition_tuple_routing = - ExecSetupPartitionTupleRouting(mtstate, rel); + ExecSetupPartitionTupleRouting(mtstate, resultRelInfo); /* * Build state for collecting transition tuples. This requires having a |