From 57fe246890ad51e166fb6a8da937e41c35d7a279 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 4 Feb 2015 16:00:34 +0200 Subject: [PATCH] Fix reference-after-free when waiting for another xact due to constraint. If an insertion or update had to wait for another transaction to finish, because there was another insertion with conflicting key in progress, we would pass a just-free'd item pointer to XactLockTableWait(). All calls to XactLockTableWait() and MultiXactIdWait() had similar issues. Some passed a pointer to a buffer in the buffer cache, after already releasing the lock. The call in EvalPlanQualFetch had already released the pin too. All but the call in execUtils.c would merely lead to reporting a bogus ctid, however (or an assertion failure, if enabled). All the callers that passed HeapTuple->t_data->t_ctid were slightly bogus anyway: if the tuple was updated (again) in the same transaction, its ctid field would point to the next tuple in the chain, not the tuple itself. Backpatch to 9.4, where the 'ctid' argument to XactLockTableWait was added (in commit f88d4cfc) --- src/backend/access/heap/heapam.c | 16 ++++++++-------- src/backend/catalog/index.c | 4 ++-- src/backend/executor/execMain.c | 2 +- src/backend/executor/execUtils.c | 4 +++- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 21e9d067b6..46060bc13e 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2717,7 +2717,7 @@ l1: { /* wait for multixact */ MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate, infomask, - relation, &tp.t_data->t_ctid, XLTW_Delete, + relation, &(tp.t_self), XLTW_Delete, NULL); LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); @@ -2744,7 +2744,7 @@ l1: else { /* wait for regular transaction to end */ - XactLockTableWait(xwait, relation, &tp.t_data->t_ctid, XLTW_Delete); + XactLockTableWait(xwait, relation, &(tp.t_self), XLTW_Delete); LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); /* @@ -3261,7 +3261,7 @@ l2: /* wait for multixact */ MultiXactIdWait((MultiXactId) xwait, mxact_status, infomask, - relation, &oldtup.t_data->t_ctid, XLTW_Update, + relation, &oldtup.t_self, XLTW_Update, &remain); LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); @@ -3341,7 +3341,7 @@ l2: */ heap_acquire_tuplock(relation, &(oldtup.t_self), *lockmode, LockWaitBlock, &have_tuple_lock); - XactLockTableWait(xwait, relation, &oldtup.t_data->t_ctid, + XactLockTableWait(xwait, relation, &oldtup.t_self, XLTW_Update); LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); @@ -4365,7 +4365,7 @@ l3: { case LockWaitBlock: MultiXactIdWait((MultiXactId) xwait, status, infomask, - relation, &tuple->t_data->t_ctid, XLTW_Lock, NULL); + relation, &tuple->t_self, XLTW_Lock, NULL); break; case LockWaitSkip: if (!ConditionalMultiXactIdWait((MultiXactId) xwait, @@ -4437,7 +4437,7 @@ l3: switch (wait_policy) { case LockWaitBlock: - XactLockTableWait(xwait, relation, &tuple->t_data->t_ctid, + XactLockTableWait(xwait, relation, &tuple->t_self, XLTW_Lock); break; case LockWaitSkip: @@ -5181,7 +5181,7 @@ l4: { LockBuffer(buf, BUFFER_LOCK_UNLOCK); XactLockTableWait(members[i].xid, rel, - &mytup.t_data->t_ctid, + &mytup.t_self, XLTW_LockUpdated); pfree(members); goto l4; @@ -5242,7 +5242,7 @@ l4: if (needwait) { LockBuffer(buf, BUFFER_LOCK_UNLOCK); - XactLockTableWait(rawxmax, rel, &mytup.t_data->t_ctid, + XactLockTableWait(rawxmax, rel, &mytup.t_self, XLTW_LockUpdated); goto l4; } diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 9bb9deb62f..f85ed93e40 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2328,7 +2328,7 @@ IndexBuildHeapRangeScan(Relation heapRelation, */ LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); XactLockTableWait(xwait, heapRelation, - &heapTuple->t_data->t_ctid, + &heapTuple->t_self, XLTW_InsertIndexUnique); CHECK_FOR_INTERRUPTS(); goto recheck; @@ -2377,7 +2377,7 @@ IndexBuildHeapRangeScan(Relation heapRelation, */ LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); XactLockTableWait(xwait, heapRelation, - &heapTuple->t_data->t_ctid, + &heapTuple->t_self, XLTW_InsertIndexUnique); CHECK_FOR_INTERRUPTS(); goto recheck; diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 20b3188dfd..33b172b1ad 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2118,7 +2118,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, { case LockWaitBlock: XactLockTableWait(SnapshotDirty.xmax, - relation, &tuple.t_data->t_ctid, + relation, &tuple.t_self, XLTW_FetchUpdated); break; case LockWaitSkip: diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 4b921fa596..022041bea4 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -1245,6 +1245,7 @@ retry: ForwardScanDirection)) != NULL) { TransactionId xwait; + ItemPointerData ctid_wait; Datum existing_values[INDEX_MAX_KEYS]; bool existing_isnull[INDEX_MAX_KEYS]; char *error_new; @@ -1306,8 +1307,9 @@ retry: if (TransactionIdIsValid(xwait)) { + ctid_wait = tup->t_data->t_ctid; index_endscan(index_scan); - XactLockTableWait(xwait, heap, &tup->t_data->t_ctid, + XactLockTableWait(xwait, heap, &ctid_wait, XLTW_RecheckExclusionConstr); goto retry; }