From da63fec7dbb0c459aa0f8753a3d30a3a6cd3d73f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 30 Nov 2012 13:55:55 -0500 Subject: [PATCH] Add missing buffer lock acquisition in GetTupleForTrigger(). If we had not been holding buffer pin continuously since the tuple was initially fetched by the UPDATE or DELETE query, it would be possible for VACUUM or a page-prune operation to move the tuple while we're trying to copy it. This would result in a garbage "old" tuple value being passed to an AFTER ROW UPDATE or AFTER ROW DELETE trigger. The preconditions for this are somewhat improbable, and the timing constraints are very tight; so it's not so surprising that this hasn't been reported from the field, even though the bug has been there a long time. Problem found by Andres Freund. Back-patch to all active branches. --- src/backend/commands/trigger.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 98b82074b5..91ef779c65 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -2662,6 +2662,16 @@ ltrmark:; buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid)); + /* + * Although we already know this tuple is valid, we must lock the + * buffer to ensure that no one has a buffer cleanup lock; otherwise + * they might move the tuple while we try to copy it. But we can + * release the lock before actually doing the heap_copytuple call, + * since holding pin is sufficient to prevent anyone from getting a + * cleanup lock they don't already hold. + */ + LockBuffer(buffer, BUFFER_LOCK_SHARE); + page = BufferGetPage(buffer); lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid)); @@ -2671,6 +2681,8 @@ ltrmark:; tuple.t_len = ItemIdGetLength(lp); tuple.t_self = *tid; tuple.t_tableOid = RelationGetRelid(relation); + + LockBuffer(buffer, BUFFER_LOCK_UNLOCK); } result = heap_copytuple(&tuple); -- GitLab