Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4a8656a

Browse files
committed
Fix mishandling of resjunk columns in ON CONFLICT ... UPDATE tlists.
It's unusual to have any resjunk columns in an ON CONFLICT ... UPDATElist, but it can happen when MULTIEXPR_SUBLINK SubPlans are present.If it happens, the ON CONFLICT UPDATE code path would end up storingtuples that include the values of the extra resjunk columns. That'sfairly harmless in the short run, but if new columns are added tothe table then the values would become accessible, possibly leadingto malfunctions if they don't match the datatypes of the new columns.This had escaped notice through a confluence of missing sanity checks,including* There's no cross-check that a tuple presented to heap_insert orheap_update matches the table rowtype. While it's difficult tocheck that fully at reasonable cost, we can easily add assertionsthat there aren't too many columns.* The output-column-assignment cases in execExprInterp.c lackedany sanity checks on the output column numbers, which seems likean oversight considering there are plenty of assertion checks oninput column numbers. Add assertions there too.* We failed to apply nodeModifyTable's ExecCheckPlanOutput() tothe ON CONFLICT UPDATE tlist. That wouldn't have caught thisspecific error, since that function is chartered to ignore resjunkcolumns; but it sure seems like a bad omission now that we've seenthis bug.In HEAD, the right way to fix this is to make the processing ofON CONFLICT UPDATE tlists work the same as regular UPDATE tlistsnow do, that is don't add "SET x = x" entries, and useExecBuildUpdateProjection to evaluate the tlist and combine it withold values of the not-set columns. This adds a little complicationto ExecBuildUpdateProjection, but allows removal of a comparableamount of now-dead code from the planner.In the back branches, the most expedient solution seems to be to(a) use an output slot for the ON CONFLICT UPDATE projection thatactually matches the target table, and then (b) invent a variant ofExecBuildProjectionInfo that can be told to not store values resultingfrom resjunk columns, so it doesn't try to store into nonexistentcolumns of the output slot. (We can't simply ignore the resjunk columnsaltogether; they have to be evaluated for MULTIEXPR_SUBLINK to work.)This works back to v10. In 9.6, projections work much differently andwe can't cheaply give them such an option. The 9.6 version of thispatch works by inserting a JunkFilter when it's necessary to get ridof resjunk columns.In addition, v11 and up have the reverse problem when trying toperform ON CONFLICT UPDATE on a partitioned table. Through afurther oversight, adjust_partition_tlist() discarded resjunk columnswhen re-ordering the ON CONFLICT UPDATE tlist to match a partition.This accidentally prevented the storing-bogus-tuples problem, butat the cost that MULTIEXPR_SUBLINK cases didn't work, typicallycrashing if more than one row has to be updated. Fix by preservingresjunk columns in that routine. (I failed to resist the temptationto add more assertions there too, and to do some minor codebeautification.)Per report from Andres Freund. Back-patch to all supported branches.Security:CVE-2021-32028
1 parent467395b commit4a8656a

File tree

8 files changed

+186
-59
lines changed

8 files changed

+186
-59
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,6 +1849,10 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
18491849
Buffervmbuffer=InvalidBuffer;
18501850
boolall_visible_cleared= false;
18511851

1852+
/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
1853+
Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
1854+
RelationGetNumberOfAttributes(relation));
1855+
18521856
/*
18531857
* Fill in tuple header fields and toast the tuple if necessary.
18541858
*
@@ -2915,6 +2919,10 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
29152919

29162920
Assert(ItemPointerIsValid(otid));
29172921

2922+
/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
2923+
Assert(HeapTupleHeaderGetNatts(newtup->t_data) <=
2924+
RelationGetNumberOfAttributes(relation));
2925+
29182926
/*
29192927
* Forbid this during a parallel operation, lest it allocate a combocid.
29202928
* Other workers might need that combocid for visibility checks, and we

‎src/backend/executor/execExpr.c

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,32 @@ ExecBuildProjectionInfo(List *targetList,
355355
TupleTableSlot*slot,
356356
PlanState*parent,
357357
TupleDescinputDesc)
358+
{
359+
returnExecBuildProjectionInfoExt(targetList,
360+
econtext,
361+
slot,
362+
true,
363+
parent,
364+
inputDesc);
365+
}
366+
367+
/*
368+
*ExecBuildProjectionInfoExt
369+
*
370+
* As above, with one additional option.
371+
*
372+
* If assignJunkEntries is true (the usual case), resjunk entries in the tlist
373+
* are not handled specially: they are evaluated and assigned to the proper
374+
* column of the result slot. If assignJunkEntries is false, resjunk entries
375+
* are evaluated, but their result is discarded without assignment.
376+
*/
377+
ProjectionInfo*
378+
ExecBuildProjectionInfoExt(List*targetList,
379+
ExprContext*econtext,
380+
TupleTableSlot*slot,
381+
boolassignJunkEntries,
382+
PlanState*parent,
383+
TupleDescinputDesc)
358384
{
359385
ProjectionInfo*projInfo=makeNode(ProjectionInfo);
360386
ExprState*state;
@@ -392,7 +418,8 @@ ExecBuildProjectionInfo(List *targetList,
392418
*/
393419
if (tle->expr!=NULL&&
394420
IsA(tle->expr,Var)&&
395-
((Var*)tle->expr)->varattno>0)
421+
((Var*)tle->expr)->varattno>0&&
422+
(assignJunkEntries|| !tle->resjunk))
396423
{
397424
/* Non-system Var, but how safe is it? */
398425
variable= (Var*)tle->expr;
@@ -456,6 +483,10 @@ ExecBuildProjectionInfo(List *targetList,
456483
ExecInitExprRec(tle->expr,state,
457484
&state->resvalue,&state->resnull);
458485

486+
/* This makes it easy to discard resjunk results when told to. */
487+
if (!assignJunkEntries&&tle->resjunk)
488+
continue;
489+
459490
/*
460491
* Column might be referenced multiple times in upper nodes, so
461492
* force value to R/O - but only if it could be an expanded datum.

‎src/backend/executor/execExprInterp.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
581581
* care of at compilation time. But see EEOP_INNER_VAR comments.
582582
*/
583583
Assert(attnum >=0&&attnum<innerslot->tts_nvalid);
584+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
584585
resultslot->tts_values[resultnum]=innerslot->tts_values[attnum];
585586
resultslot->tts_isnull[resultnum]=innerslot->tts_isnull[attnum];
586587

@@ -597,6 +598,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
597598
* care of at compilation time. But see EEOP_INNER_VAR comments.
598599
*/
599600
Assert(attnum >=0&&attnum<outerslot->tts_nvalid);
601+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
600602
resultslot->tts_values[resultnum]=outerslot->tts_values[attnum];
601603
resultslot->tts_isnull[resultnum]=outerslot->tts_isnull[attnum];
602604

@@ -613,6 +615,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
613615
* care of at compilation time. But see EEOP_INNER_VAR comments.
614616
*/
615617
Assert(attnum >=0&&attnum<scanslot->tts_nvalid);
618+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
616619
resultslot->tts_values[resultnum]=scanslot->tts_values[attnum];
617620
resultslot->tts_isnull[resultnum]=scanslot->tts_isnull[attnum];
618621

@@ -623,6 +626,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
623626
{
624627
intresultnum=op->d.assign_tmp.resultnum;
625628

629+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
626630
resultslot->tts_values[resultnum]=state->resvalue;
627631
resultslot->tts_isnull[resultnum]=state->resnull;
628632

@@ -633,6 +637,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
633637
{
634638
intresultnum=op->d.assign_tmp.resultnum;
635639

640+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
636641
resultslot->tts_isnull[resultnum]=state->resnull;
637642
if (!resultslot->tts_isnull[resultnum])
638643
resultslot->tts_values[resultnum]=
@@ -2074,8 +2079,10 @@ ExecJustAssignVarImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull)
20742079
*
20752080
* Since we use slot_getattr(), we don't need to implement the FETCHSOME
20762081
* step explicitly, and we also needn't Assert that the attnum is in range
2077-
* --- slot_getattr() will take care of any problems.
2082+
* --- slot_getattr() will take care of any problems. Nonetheless, check
2083+
* that resultnum is in range.
20782084
*/
2085+
Assert(resultnum >=0&&resultnum<outslot->tts_tupleDescriptor->natts);
20792086
outslot->tts_values[resultnum]=
20802087
slot_getattr(inslot,attnum,&outslot->tts_isnull[resultnum]);
20812088
return0;
@@ -2207,6 +2214,7 @@ ExecJustAssignVarVirtImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull
22072214
Assert(TTS_IS_VIRTUAL(inslot));
22082215
Assert(TTS_FIXED(inslot));
22092216
Assert(attnum >=0&&attnum<inslot->tts_nvalid);
2217+
Assert(resultnum >=0&&resultnum<outslot->tts_tupleDescriptor->natts);
22102218

22112219
outslot->tts_values[resultnum]=inslot->tts_values[attnum];
22122220
outslot->tts_isnull[resultnum]=inslot->tts_isnull[attnum];

‎src/backend/executor/execPartition.c

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -785,21 +785,22 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
785785
*/
786786
if (node->onConflictAction==ONCONFLICT_UPDATE)
787787
{
788+
OnConflictSetState*onconfl=makeNode(OnConflictSetState);
788789
TupleConversionMap*map;
789790

790791
map=leaf_part_rri->ri_PartitionInfo->pi_RootToPartitionMap;
791792

792793
Assert(node->onConflictSet!=NIL);
793794
Assert(rootResultRelInfo->ri_onConflict!=NULL);
794795

795-
leaf_part_rri->ri_onConflict=makeNode(OnConflictSetState);
796+
leaf_part_rri->ri_onConflict=onconfl;
796797

797798
/*
798799
* Need a separate existing slot for each partition, as the
799800
* partition could be of a different AM, even if the tuple
800801
* descriptors match.
801802
*/
802-
leaf_part_rri->ri_onConflict->oc_Existing=
803+
onconfl->oc_Existing=
803804
table_slot_create(leaf_part_rri->ri_RelationDesc,
804805
&mtstate->ps.state->es_tupleTable);
805806

@@ -819,17 +820,16 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
819820
* Projections and where clauses themselves don't store state
820821
* / are independent of the underlying storage.
821822
*/
822-
leaf_part_rri->ri_onConflict->oc_ProjSlot=
823+
onconfl->oc_ProjSlot=
823824
rootResultRelInfo->ri_onConflict->oc_ProjSlot;
824-
leaf_part_rri->ri_onConflict->oc_ProjInfo=
825+
onconfl->oc_ProjInfo=
825826
rootResultRelInfo->ri_onConflict->oc_ProjInfo;
826-
leaf_part_rri->ri_onConflict->oc_WhereClause=
827+
onconfl->oc_WhereClause=
827828
rootResultRelInfo->ri_onConflict->oc_WhereClause;
828829
}
829830
else
830831
{
831832
List*onconflset;
832-
TupleDesctupDesc;
833833
boolfound_whole_row;
834834

835835
/*
@@ -839,7 +839,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
839839
* pseudo-relation (INNER_VAR), and second to handle the main
840840
* target relation (firstVarno).
841841
*/
842-
onconflset=(List*)copyObject((Node*)node->onConflictSet);
842+
onconflset=copyObject(node->onConflictSet);
843843
if (part_attmap==NULL)
844844
part_attmap=
845845
build_attrmap_by_name(RelationGetDescr(partrel),
@@ -859,20 +859,19 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
859859
&found_whole_row);
860860
/* We ignore the value of found_whole_row. */
861861

862-
/* Finally,adjust this tlist to match the partition. */
862+
/* Finally,reorder the tlist to match the partition. */
863863
onconflset=adjust_partition_tlist(onconflset,map);
864864

865865
/* create the tuple slot for the UPDATE SET projection */
866-
tupDesc=ExecTypeFromTL(onconflset);
867-
leaf_part_rri->ri_onConflict->oc_ProjSlot=
868-
ExecInitExtraTupleSlot(mtstate->ps.state,tupDesc,
869-
&TTSOpsVirtual);
866+
onconfl->oc_ProjSlot=
867+
table_slot_create(partrel,
868+
&mtstate->ps.state->es_tupleTable);
870869

871870
/* build UPDATE SET projection state */
872-
leaf_part_rri->ri_onConflict->oc_ProjInfo=
873-
ExecBuildProjectionInfo(onconflset,econtext,
874-
leaf_part_rri->ri_onConflict->oc_ProjSlot,
875-
&mtstate->ps,partrelDesc);
871+
onconfl->oc_ProjInfo=
872+
ExecBuildProjectionInfoExt(onconflset,econtext,
873+
onconfl->oc_ProjSlot, false,
874+
&mtstate->ps,partrelDesc);
876875

877876
/*
878877
* If there is a WHERE clause, initialize state where it will
@@ -899,7 +898,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
899898
RelationGetForm(partrel)->reltype,
900899
&found_whole_row);
901900
/* We ignore the value of found_whole_row. */
902-
leaf_part_rri->ri_onConflict->oc_WhereClause=
901+
onconfl->oc_WhereClause=
903902
ExecInitQual((List*)clause,&mtstate->ps);
904903
}
905904
}
@@ -1487,18 +1486,15 @@ ExecBuildSlotPartitionKeyDescription(Relation rel,
14871486

14881487
/*
14891488
* adjust_partition_tlist
1490-
*Adjust the targetlist entries for a given partition to account for
1491-
*attributedifferences between parent and the partition
1489+
*Re-order the targetlist entries for a given partition to account for
1490+
*column positiondifferences betweentheparent and the partition.
14921491
*
1493-
* The expressions have already been fixed, but here we fix the list to make
1494-
* target resnos match the partition's attribute numbers. This results in a
1495-
* copy of the original target list in which the entries appear in resno
1496-
* order, including both the existing entries (that may have their resno
1497-
* changed in-place) and the newly added entries for columns that don't exist
1498-
* in the parent.
1492+
* The expressions have already been fixed, but we must now re-order the
1493+
* entries in case the partition has different column order, and possibly
1494+
* add or remove dummy entries for dropped columns.
14991495
*
1500-
*Scribbles on the input tlist, so callers must make suretomake a copy
1501-
*before passing it to us.
1496+
*Although a new List is returned, this feels freetoscribble on resno
1497+
*fields of the given tlist, so that should be a working copy.
15021498
*/
15031499
staticList*
15041500
adjust_partition_tlist(List*tlist,TupleConversionMap*map)
@@ -1507,32 +1503,36 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
15071503
TupleDesctupdesc=map->outdesc;
15081504
AttrMap*attrMap=map->attrMap;
15091505
AttrNumberattrno;
1506+
ListCell*lc;
15101507

15111508
Assert(tupdesc->natts==attrMap->maplen);
15121509
for (attrno=1;attrno <=tupdesc->natts;attrno++)
15131510
{
15141511
Form_pg_attributeatt_tup=TupleDescAttr(tupdesc,attrno-1);
1512+
AttrNumberparentattrno=attrMap->attnums[attrno-1];
15151513
TargetEntry*tle;
15161514

1517-
if (attrMap->attnums[attrno-1]!=InvalidAttrNumber)
1515+
if (parentattrno!=InvalidAttrNumber)
15181516
{
1519-
Assert(!att_tup->attisdropped);
1520-
15211517
/*
15221518
* Use the corresponding entry from the parent's tlist, adjusting
1523-
* the resnothe match the partition's attno.
1519+
* the resnoto match the partition's attno.
15241520
*/
1525-
tle= (TargetEntry*)list_nth(tlist,attrMap->attnums[attrno-1]-1);
1521+
Assert(!att_tup->attisdropped);
1522+
tle= (TargetEntry*)list_nth(tlist,parentattrno-1);
1523+
Assert(!tle->resjunk);
1524+
Assert(tle->resno==parentattrno);
15261525
tle->resno=attrno;
15271526
}
15281527
else
15291528
{
1530-
Const*expr;
1531-
15321529
/*
15331530
* For a dropped attribute in the partition, generate a dummy
1534-
* entry with resno matching the partition's attno.
1531+
* entry with resno matching the partition's attno. This should
1532+
* match what expand_targetlist() does.
15351533
*/
1534+
Const*expr;
1535+
15361536
Assert(att_tup->attisdropped);
15371537
expr=makeConst(INT4OID,
15381538
-1,
@@ -1550,6 +1550,18 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
15501550
new_tlist=lappend(new_tlist,tle);
15511551
}
15521552

1553+
/* Finally, attach any resjunk entries to the end of the new tlist */
1554+
foreach(lc,tlist)
1555+
{
1556+
TargetEntry*tle= (TargetEntry*)lfirst(lc);
1557+
1558+
if (tle->resjunk)
1559+
{
1560+
tle->resno=list_length(new_tlist)+1;
1561+
new_tlist=lappend(new_tlist,tle);
1562+
}
1563+
}
1564+
15531565
returnnew_tlist;
15541566
}
15551567

‎src/backend/executor/nodeModifyTable.c

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2608,9 +2608,9 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
26082608
*/
26092609
if (node->onConflictAction==ONCONFLICT_UPDATE)
26102610
{
2611+
OnConflictSetState*onconfl=makeNode(OnConflictSetState);
26112612
ExprContext*econtext;
26122613
TupleDescrelationDesc;
2613-
TupleDesctupDesc;
26142614

26152615
/* insert may only have one plan, inheritance is not expanded */
26162616
Assert(nplans==1);
@@ -2623,10 +2623,10 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
26232623
relationDesc=resultRelInfo->ri_RelationDesc->rd_att;
26242624

26252625
/* create state for DO UPDATE SET operation */
2626-
resultRelInfo->ri_onConflict=makeNode(OnConflictSetState);
2626+
resultRelInfo->ri_onConflict=onconfl;
26272627

26282628
/* initialize slot for the existing tuple */
2629-
resultRelInfo->ri_onConflict->oc_Existing=
2629+
onconfl->oc_Existing=
26302630
table_slot_create(resultRelInfo->ri_RelationDesc,
26312631
&mtstate->ps.state->es_tupleTable);
26322632

@@ -2636,17 +2636,25 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
26362636
* into the table, and for RETURNING processing - which may access
26372637
* system attributes.
26382638
*/
2639-
tupDesc=ExecTypeFromTL((List*)node->onConflictSet);
2640-
resultRelInfo->ri_onConflict->oc_ProjSlot=
2641-
ExecInitExtraTupleSlot(mtstate->ps.state,tupDesc,
2642-
table_slot_callbacks(resultRelInfo->ri_RelationDesc));
2639+
onconfl->oc_ProjSlot=
2640+
table_slot_create(resultRelInfo->ri_RelationDesc,
2641+
&mtstate->ps.state->es_tupleTable);
2642+
2643+
/*
2644+
* The onConflictSet tlist should already have been adjusted to emit
2645+
* the table's exact column list. It could also contain resjunk
2646+
* columns, which should be evaluated but not included in the
2647+
* projection result.
2648+
*/
2649+
ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
2650+
node->onConflictSet);
26432651

26442652
/* build UPDATE SET projection state */
2645-
resultRelInfo->ri_onConflict->oc_ProjInfo=
2646-
ExecBuildProjectionInfo(node->onConflictSet,econtext,
2647-
resultRelInfo->ri_onConflict->oc_ProjSlot,
2648-
&mtstate->ps,
2649-
relationDesc);
2653+
onconfl->oc_ProjInfo=
2654+
ExecBuildProjectionInfoExt(node->onConflictSet,econtext,
2655+
onconfl->oc_ProjSlot, false,
2656+
&mtstate->ps,
2657+
relationDesc);
26502658

26512659
/* initialize state to evaluate the WHERE clause, if any */
26522660
if (node->onConflictWhere)
@@ -2655,7 +2663,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
26552663

26562664
qualexpr=ExecInitQual((List*)node->onConflictWhere,
26572665
&mtstate->ps);
2658-
resultRelInfo->ri_onConflict->oc_WhereClause=qualexpr;
2666+
onconfl->oc_WhereClause=qualexpr;
26592667
}
26602668
}
26612669

‎src/include/executor/executor.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,12 @@ extern ProjectionInfo *ExecBuildProjectionInfo(List *targetList,
271271
TupleTableSlot*slot,
272272
PlanState*parent,
273273
TupleDescinputDesc);
274+
externProjectionInfo*ExecBuildProjectionInfoExt(List*targetList,
275+
ExprContext*econtext,
276+
TupleTableSlot*slot,
277+
boolassignJunkEntries,
278+
PlanState*parent,
279+
TupleDescinputDesc);
274280
externExprState*ExecPrepareExpr(Expr*node,EState*estate);
275281
externExprState*ExecPrepareQual(List*qual,EState*estate);
276282
externExprState*ExecPrepareCheck(List*qual,EState*estate);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp