From 2b53793ca2c32ca8b3d26dc8f0ee72c3a623ed69 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 2 Feb 2015 16:24:58 +0200 Subject: [PATCH 1/1] 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 an 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 21e9d06..46060bc 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 9bb9deb..f85ed93 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 20b3188..33b172b 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 4b921fa..022041b 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; } -- 2.1.4