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)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 21e9d067b6fc6b47774dce8c9e872e70ead78038..46060bc13e23be138f699cdcbac56d85e29c0378 100644 (file)
--- 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 9bb9deb62f98ad5d36bb27fd639681e1056f4ab0..f85ed93e401153ab94b8164b000fd0c0eedf8d97 100644 (file)
--- 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 20b3188dfdc1fd41d3adf9194cdb34c323bcfd60..33b172b1ad22efecff26b719a586f99a8c4461d8 100644 (file)
--- 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 4b921fa5965f56cb3bd0bdea36fe931eb435426c..022041bea44c159acc09ecf13994207c00bae2a1 100644 (file)
--- 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;
        }