From 48a565d78ba9413317d6095010cf622e9d6a36eb 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 fc7472d265..2209db1d96 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2742,7 +2742,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); @@ -2769,7 +2769,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); /* @@ -3298,7 +3298,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); @@ -3378,7 +3378,7 @@ l2: */ heap_acquire_tuplock(relation, &(oldtup.t_self), *lockmode, false, &have_tuple_lock); - XactLockTableWait(xwait, relation, &oldtup.t_data->t_ctid, + XactLockTableWait(xwait, relation, &oldtup.t_self, XLTW_Update); LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); @@ -4396,7 +4396,7 @@ l3: } else MultiXactIdWait((MultiXactId) xwait, status, infomask, - relation, &tuple->t_data->t_ctid, + relation, &tuple->t_self, XLTW_Lock, NULL); /* if there are updates, follow the update chain */ @@ -4452,7 +4452,7 @@ l3: RelationGetRelationName(relation)))); } else - XactLockTableWait(xwait, relation, &tuple->t_data->t_ctid, + XactLockTableWait(xwait, relation, &tuple->t_self, XLTW_Lock); /* if there are updates, follow the update chain */ @@ -5168,7 +5168,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; @@ -5229,7 +5229,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 f691dd30c1..adb081ab81 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2288,7 +2288,7 @@ IndexBuildHeapScan(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; @@ -2337,7 +2337,7 @@ IndexBuildHeapScan(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 bf82b449d5..a5d4a820ff 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2104,7 +2104,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, bool noWait, } else XactLockTableWait(SnapshotDirty.xmax, - relation, &tuple.t_data->t_ctid, + relation, &tuple.t_self, XLTW_FetchUpdated); continue; /* loop back to repeat heap_fetch */ } diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 5c08501ee7..756b9641e3 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.40.0