This is an automated email from the ASF dual-hosted git repository. leborchuk pushed a commit to branch REL_2_STABLE in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 72fe336300907b9d7088a16ac9fae43330267e8f Author: Tom Lane <[email protected]> AuthorDate: Sun Jan 14 12:38:41 2024 -0500 Prevent access to an unpinned buffer in BEFORE ROW UPDATE triggers. When ExecBRUpdateTriggers switches to a new target tuple as a result of the EvalPlanQual logic, it must form a new proposed update tuple. Since commit 86dc90056, that tuple (the result of ExecGetUpdateNewTuple) has been a virtual tuple that might contain pointers to by-ref fields of the new target tuple (in "oldslot"). However, immediately after that we materialize oldslot, causing it to drop its buffer pin, whereupon the by-ref pointers are unsafe to use. This is a live bug only when the new target tuple is in a different page than the original target tuple, since we do still hold a pin on the original one. (Before 86dc90056, there was no bug because the EPQ plantree would hold a pin on the new target tuple; but now that's not assured.) To fix, forcibly materialize the new tuple before we materialize oldslot. This costs nothing since we would have done that shortly anyway. The real-world impact of this is probably minimal. A visible failure could occur if the new target tuple's buffer were recycled for some other page in the short interval before we materialize newslot within the trigger-calling loop; but that's quite unlikely given that we'd just touched that page. There's a larger hazard that some other process could prune and repack that page within the window. We have lock on the new target tuple, but that wouldn't prevent it being moved on the page. Alexander Lakhin and Tom Lane, per bug #17798 from Alexander Lakhin. Back-patch to v14 where 86dc90056 came in. Discussion: https://postgr.es/m/[email protected] (cherry picked from commit c7edaeec50546e10b15e27de94fdea1252cbe76b) --- src/backend/commands/trigger.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 45794a55753..a49908ad5d9 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -2830,10 +2830,6 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, * received in newslot. Neither we nor our callers have any further * interest in the passed-in tuple, so it's okay to overwrite newslot * with the newer data. - * - * (Typically, newslot was also generated by ExecGetUpdateNewTuple, so - * that epqslot_clean will be that same slot and the copy step below - * is not needed.) */ if (epqslot_candidate != NULL) { @@ -2842,14 +2838,36 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, epqslot_clean = ExecGetUpdateNewTuple(relinfo, epqslot_candidate, oldslot); - if (newslot != epqslot_clean) + /* + * Typically, the caller's newslot was also generated by + * ExecGetUpdateNewTuple, so that epqslot_clean will be the same + * slot and copying is not needed. But do the right thing if it + * isn't. + */ + if (unlikely(newslot != epqslot_clean)) ExecCopySlot(newslot, epqslot_clean); + + /* + * At this point newslot contains a virtual tuple that may + * reference some fields of oldslot's tuple in some disk buffer. + * If that tuple is in a different page than the original target + * tuple, then our only pin on that buffer is oldslot's, and we're + * about to release it. Hence we'd better materialize newslot to + * ensure it doesn't contain references into an unpinned buffer. + * (We'd materialize it below anyway, but too late for safety.) + */ + ExecMaterializeSlot(newslot); } + /* + * Here we convert oldslot to a materialized slot holding trigtuple. + * Neither slot passed to the triggers will hold any buffer pin. + */ trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig); } else { + /* Put the FDW-supplied tuple into oldslot to unify the cases */ ExecForceStoreHeapTuple(fdw_trigtuple, oldslot, false); trigtuple = fdw_trigtuple; } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
