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

Commit049e1e2

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 parentf02b908 commit049e1e2

File tree

15 files changed

+265
-205
lines changed

15 files changed

+265
-205
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2070,6 +2070,10 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
20702070
boolall_frozen_set= false;
20712071
uint8vmstatus=0;
20722072

2073+
/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
2074+
Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
2075+
RelationGetNumberOfAttributes(relation));
2076+
20732077
/*
20742078
* Fill in tuple header fields and toast the tuple if necessary.
20752079
*
@@ -3255,6 +3259,10 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
32553259

32563260
Assert(ItemPointerIsValid(otid));
32573261

3262+
/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
3263+
Assert(HeapTupleHeaderGetNatts(newtup->t_data) <=
3264+
RelationGetNumberOfAttributes(relation));
3265+
32583266
/*
32593267
* Forbid this during a parallel operation, lest it allocate a combo CID.
32603268
* Other workers might need that combo CID for visibility checks, and we

‎src/backend/executor/execExpr.c

Lines changed: 83 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -485,14 +485,21 @@ ExecBuildProjectionInfo(List *targetList,
485485
* be stored into the given tuple slot. (Caller must have ensured that tuple
486486
* slot has a descriptor matching the target rel!)
487487
*
488-
* subTargetList is the tlist of the subplan node feeding ModifyTable.
489-
* We use this mainly to cross-check that the expressions being assigned
490-
* are of the correct types. The values from this tlist are assumed to be
491-
* available from the "outer" tuple slot. They are assigned to target columns
492-
* listed in the corresponding targetColnos elements. (Only non-resjunk tlist
493-
* entries are assigned.) Columns not listed in targetColnos are filled from
494-
* the UPDATE's old tuple, which is assumed to be available in the "scan"
495-
* tuple slot.
488+
* When evalTargetList is false, targetList contains the UPDATE ... SET
489+
* expressions that have already been computed by a subplan node; the values
490+
* from this tlist are assumed to be available in the "outer" tuple slot.
491+
* When evalTargetList is true, targetList contains the UPDATE ... SET
492+
* expressions that must be computed (which could contain references to
493+
* the outer, inner, or scan tuple slots).
494+
*
495+
* In either case, targetColnos contains a list of the target column numbers
496+
* corresponding to the non-resjunk entries of targetList. The tlist values
497+
* are assigned into these columns of the result tuple slot. Target columns
498+
* not listed in targetColnos are filled from the UPDATE's old tuple, which
499+
* is assumed to be available in the "scan" tuple slot.
500+
*
501+
* targetList can also contain resjunk columns. These must be evaluated
502+
* if evalTargetList is true, but their values are discarded.
496503
*
497504
* relDesc must describe the relation we intend to update.
498505
*
@@ -503,7 +510,8 @@ ExecBuildProjectionInfo(List *targetList,
503510
* ExecCheckPlanOutput, so we must do our safety checks here.
504511
*/
505512
ProjectionInfo*
506-
ExecBuildUpdateProjection(List*subTargetList,
513+
ExecBuildUpdateProjection(List*targetList,
514+
boolevalTargetList,
507515
List*targetColnos,
508516
TupleDescrelDesc,
509517
ExprContext*econtext,
@@ -525,19 +533,22 @@ ExecBuildUpdateProjection(List *subTargetList,
525533
/* We embed ExprState into ProjectionInfo instead of doing extra palloc */
526534
projInfo->pi_state.tag=T_ExprState;
527535
state=&projInfo->pi_state;
528-
state->expr=NULL;/* not used */
536+
if (evalTargetList)
537+
state->expr= (Expr*)targetList;
538+
else
539+
state->expr=NULL;/* not used */
529540
state->parent=parent;
530541
state->ext_params=NULL;
531542

532543
state->resultslot=slot;
533544

534545
/*
535-
* Examine thesubplan tlistto see how many non-junk columns there are,
536-
*andto verify that the non-junk columns come before the junk ones.
546+
* Examine thetargetListto see how many non-junk columns there are, and
547+
* to verify that the non-junk columns come before the junk ones.
537548
*/
538549
nAssignableCols=0;
539550
sawJunk= false;
540-
foreach(lc,subTargetList)
551+
foreach(lc,targetList)
541552
{
542553
TargetEntry*tle=lfirst_node(TargetEntry,lc);
543554

@@ -569,12 +580,10 @@ ExecBuildUpdateProjection(List *subTargetList,
569580
}
570581

571582
/*
572-
* Wewant to insert EEOP_*_FETCHSOME steps to ensure theouter and scan
573-
*tuples aresufficiently deconstructed.Outertupleis easy, but for
574-
*scan tuple we must find out the last old column we need.
583+
* Weneed to insert EEOP_*_FETCHSOME steps to ensure theinput tuples are
584+
* sufficiently deconstructed.The scantuplemust be deconstructed at
585+
*least as far as the last old column we need.
575586
*/
576-
deform.last_outer=nAssignableCols;
577-
578587
for (intattnum=relDesc->natts;attnum>0;attnum--)
579588
{
580589
Form_pg_attributeattr=TupleDescAttr(relDesc,attnum-1);
@@ -587,15 +596,26 @@ ExecBuildUpdateProjection(List *subTargetList,
587596
break;
588597
}
589598

599+
/*
600+
* If we're actually evaluating the tlist, incorporate its input
601+
* requirements too; otherwise, we'll just need to fetch the appropriate
602+
* number of columns of the "outer" tuple.
603+
*/
604+
if (evalTargetList)
605+
get_last_attnums_walker((Node*)targetList,&deform);
606+
else
607+
deform.last_outer=nAssignableCols;
608+
590609
ExecPushExprSlots(state,&deform);
591610

592611
/*
593-
* Now generate code to fetch data from the outer tuple, incidentally
594-
* validating that it'll be of the right type. The checks above ensure
595-
* that the forboth() will iterate over exactly the non-junk columns.
612+
* Now generate code to evaluate the tlist's assignable expressions or
613+
* fetch them from the outer tuple, incidentally validating that they'll
614+
* be of the right data type. The checks above ensure that the forboth()
615+
* will iterate over exactly the non-junk columns.
596616
*/
597617
outerattnum=0;
598-
forboth(lc,subTargetList,lc2,targetColnos)
618+
forboth(lc,targetList,lc2,targetColnos)
599619
{
600620
TargetEntry*tle=lfirst_node(TargetEntry,lc);
601621
AttrNumbertargetattnum=lfirst_int(lc2);
@@ -628,13 +648,47 @@ ExecBuildUpdateProjection(List *subTargetList,
628648
targetattnum,
629649
format_type_be(exprType((Node*)tle->expr)))));
630650

631-
/*
632-
* OK, build an outer-tuple reference.
633-
*/
634-
scratch.opcode=EEOP_ASSIGN_OUTER_VAR;
635-
scratch.d.assign_var.attnum=outerattnum++;
636-
scratch.d.assign_var.resultnum=targetattnum-1;
637-
ExprEvalPushStep(state,&scratch);
651+
/* OK, generate code to perform the assignment. */
652+
if (evalTargetList)
653+
{
654+
/*
655+
* We must evaluate the TLE's expression and assign it. We do not
656+
* bother jumping through hoops for "safe" Vars like
657+
* ExecBuildProjectionInfo does; this is a relatively less-used
658+
* path and it doesn't seem worth expending code for that.
659+
*/
660+
ExecInitExprRec(tle->expr,state,
661+
&state->resvalue,&state->resnull);
662+
/* Needn't worry about read-only-ness here, either. */
663+
scratch.opcode=EEOP_ASSIGN_TMP;
664+
scratch.d.assign_tmp.resultnum=targetattnum-1;
665+
ExprEvalPushStep(state,&scratch);
666+
}
667+
else
668+
{
669+
/* Just assign from the outer tuple. */
670+
scratch.opcode=EEOP_ASSIGN_OUTER_VAR;
671+
scratch.d.assign_var.attnum=outerattnum;
672+
scratch.d.assign_var.resultnum=targetattnum-1;
673+
ExprEvalPushStep(state,&scratch);
674+
}
675+
outerattnum++;
676+
}
677+
678+
/*
679+
* If we're evaluating the tlist, must evaluate any resjunk columns too.
680+
* (This matters for things like MULTIEXPR_SUBLINK SubPlans.)
681+
*/
682+
if (evalTargetList)
683+
{
684+
for_each_cell(lc,targetList,lc)
685+
{
686+
TargetEntry*tle=lfirst_node(TargetEntry,lc);
687+
688+
Assert(tle->resjunk);
689+
ExecInitExprRec(tle->expr,state,
690+
&state->resvalue,&state->resnull);
691+
}
638692
}
639693

640694
/*

‎src/backend/executor/execExprInterp.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
626626
* care of at compilation time. But see EEOP_INNER_VAR comments.
627627
*/
628628
Assert(attnum >=0&&attnum<innerslot->tts_nvalid);
629+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
629630
resultslot->tts_values[resultnum]=innerslot->tts_values[attnum];
630631
resultslot->tts_isnull[resultnum]=innerslot->tts_isnull[attnum];
631632

@@ -642,6 +643,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
642643
* care of at compilation time. But see EEOP_INNER_VAR comments.
643644
*/
644645
Assert(attnum >=0&&attnum<outerslot->tts_nvalid);
646+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
645647
resultslot->tts_values[resultnum]=outerslot->tts_values[attnum];
646648
resultslot->tts_isnull[resultnum]=outerslot->tts_isnull[attnum];
647649

@@ -658,6 +660,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
658660
* care of at compilation time. But see EEOP_INNER_VAR comments.
659661
*/
660662
Assert(attnum >=0&&attnum<scanslot->tts_nvalid);
663+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
661664
resultslot->tts_values[resultnum]=scanslot->tts_values[attnum];
662665
resultslot->tts_isnull[resultnum]=scanslot->tts_isnull[attnum];
663666

@@ -668,6 +671,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
668671
{
669672
intresultnum=op->d.assign_tmp.resultnum;
670673

674+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
671675
resultslot->tts_values[resultnum]=state->resvalue;
672676
resultslot->tts_isnull[resultnum]=state->resnull;
673677

@@ -678,6 +682,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
678682
{
679683
intresultnum=op->d.assign_tmp.resultnum;
680684

685+
Assert(resultnum >=0&&resultnum<resultslot->tts_tupleDescriptor->natts);
681686
resultslot->tts_isnull[resultnum]=state->resnull;
682687
if (!resultslot->tts_isnull[resultnum])
683688
resultslot->tts_values[resultnum]=
@@ -2091,8 +2096,10 @@ ExecJustAssignVarImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull)
20912096
*
20922097
* Since we use slot_getattr(), we don't need to implement the FETCHSOME
20932098
* step explicitly, and we also needn't Assert that the attnum is in range
2094-
* --- slot_getattr() will take care of any problems.
2099+
* --- slot_getattr() will take care of any problems. Nonetheless, check
2100+
* that resultnum is in range.
20952101
*/
2102+
Assert(resultnum >=0&&resultnum<outslot->tts_tupleDescriptor->natts);
20962103
outslot->tts_values[resultnum]=
20972104
slot_getattr(inslot,attnum,&outslot->tts_isnull[resultnum]);
20982105
return0;
@@ -2224,6 +2231,7 @@ ExecJustAssignVarVirtImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull
22242231
Assert(TTS_IS_VIRTUAL(inslot));
22252232
Assert(TTS_FIXED(inslot));
22262233
Assert(attnum >=0&&attnum<inslot->tts_nvalid);
2234+
Assert(resultnum >=0&&resultnum<outslot->tts_tupleDescriptor->natts);
22272235

22282236
outslot->tts_values[resultnum]=inslot->tts_values[attnum];
22292237
outslot->tts_isnull[resultnum]=inslot->tts_isnull[attnum];

0 commit comments

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