Skip to content

Commit

Permalink
Fix oversight in handling of modifiedCols since f245236
Browse files Browse the repository at this point in the history
Commit f245236 fixed a memory leak by moving the modifiedCols bitmap
into the per-row memory context. In the case of AFTER UPDATE triggers,
the bitmap is however referenced from an event kept until the end of the
query, resulting in a use-after-free bug.

Fixed by copying the bitmap into the AfterTriggerEvents memory context,
which is the one where we keep the trigger events. There's only one
place that needs to do the copy, but the memory context may not exist
yet. Doing that in a separate function seems more readable.

Report by Alexander Pyhalov, fix by me. Backpatch to 13, where the
bitmap was added to the event by commit 71d60e2.

Reported-by: Alexander Pyhalov
Backpatch-through: 13
Discussion: https://postgr.es/m/acddb17c89b0d6cb940eaeda18c08bbe@postgrespro.ru
  • Loading branch information
tvondra committed Jul 2, 2023
1 parent 3ce761d commit 984c23f
Showing 1 changed file with 32 additions and 1 deletion.
33 changes: 32 additions & 1 deletion src/backend/commands/trigger.c
Expand Up @@ -3610,6 +3610,37 @@ afterTriggerCheckState(AfterTriggerShared evtshared)
return ((evtshared->ats_event & AFTER_TRIGGER_INITDEFERRED) != 0);
}

/* ----------
* afterTriggerCopyBitmap()
*
* Copy bitmap into AfterTriggerEvents memory context, which is where the after
* trigger events are kept.
* ----------
*/
static Bitmapset *
afterTriggerCopyBitmap(Bitmapset *src)
{
Bitmapset *dst;
MemoryContext oldcxt;

if (src == NULL)
return NULL;

/* Create event context if we didn't already */
if (afterTriggers.event_cxt == NULL)
afterTriggers.event_cxt =
AllocSetContextCreate(TopTransactionContext,
"AfterTriggerEvents",
ALLOCSET_DEFAULT_SIZES);

oldcxt = MemoryContextSwitchTo(afterTriggers.event_cxt);

dst = bms_copy(src);

MemoryContextSwitchTo(oldcxt);

return dst;
}

/* ----------
* afterTriggerAddEvent()
Expand Down Expand Up @@ -5726,7 +5757,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
new_shared.ats_table = transition_capture->tcs_private;
else
new_shared.ats_table = NULL;
new_shared.ats_modifiedcols = modifiedCols;
new_shared.ats_modifiedcols = afterTriggerCopyBitmap(modifiedCols);

afterTriggerAddEvent(&afterTriggers.query_stack[afterTriggers.query_depth].events,
&new_event, &new_shared);
Expand Down

0 comments on commit 984c23f

Please sign in to comment.