Fix bogus initialization of replication origin shared memory state.
authorTom Lane <[email protected]>
Fri, 15 May 2020 23:05:40 +0000 (19:05 -0400)
committerTom Lane <[email protected]>
Fri, 15 May 2020 23:05:40 +0000 (19:05 -0400)
The previous coding zeroed out offsetof(ReplicationStateCtl, states)
more bytes than it was entitled to, as a consequence of starting the
zeroing from the wrong pointer (or, if you prefer, using the wrong
calculation of how much to zero).

It's unsurprising that this has not caused any reported problems,
since it can be expected that the newly-allocated block is at the end
of what we've used in shared memory, and we always make the shmem
block substantially bigger than minimally necessary.  Nonetheless,
this is wrong and it could bite us someday; plus it's a dangerous
model for somebody to copy.

This dates back to the introduction of this code (commit 5aa235042),
so back-patch to all supported branches.

src/backend/replication/logical/origin.c

index 16e62e269ddd143b5159b67cdccecbdd5e171fa4..9363487e220a447b0b4877c9ee9976c3695071e7 100644 (file)
@@ -144,6 +144,7 @@ typedef struct ReplicationStateCtl
 {
    int         tranche_id;
    LWLockTranche tranche;
+   /* Array of length max_replication_slots */
    ReplicationState states[FLEXIBLE_ARRAY_MEMBER];
 } ReplicationStateCtl;
 
@@ -160,6 +161,10 @@ TimestampTz replorigin_session_origin_timestamp = 0;
  * max_replication_slots?
  */
 static ReplicationState *replication_states;
+
+/*
+ * Actual shared memory block (replication_states[] is now part of this).
+ */
 static ReplicationStateCtl *replication_states_ctl;
 
 /*
@@ -447,7 +452,7 @@ ReplicationOriginShmemSize(void)
    /*
     * XXX: max_replication_slots is arguably the wrong thing to use, as here
     * we keep the replay state of *remote* transactions. But for now it seems
-    * sufficient to reuse it, lest we introduce a separate guc.
+    * sufficient to reuse it, rather than introduce a separate GUC.
     */
    if (max_replication_slots == 0)
        return size;
@@ -477,6 +482,8 @@ ReplicationOriginShmemInit(void)
    {
        int         i;
 
+       MemSet(replication_states_ctl, 0, ReplicationOriginShmemSize());
+
        replication_states_ctl->tranche_id = LWLockNewTrancheId();
        replication_states_ctl->tranche.name = "ReplicationOrigins";
        replication_states_ctl->tranche.array_base =
@@ -484,8 +491,6 @@ ReplicationOriginShmemInit(void)
        replication_states_ctl->tranche.array_stride =
            sizeof(ReplicationState);
 
-       MemSet(replication_states, 0, ReplicationOriginShmemSize());
-
        for (i = 0; i < max_replication_slots; i++)
            LWLockInitialize(&replication_states[i].lock,
                             replication_states_ctl->tranche_id);