From 4295b277952a46378f01211bd37075f20223aadc Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Date: Mon, 25 Nov 2019 15:21:38 +0100
Subject: [PATCH] Fix subscriber invalid memory access on DDL

Before this patch, the attrmap structure mapping the local fields
to remote ones for a subscribed relation was rebuild before handling
pending invalidation messages and potential relcache updates. This
was leading to an invalid memory access by overflowing the attrmap
when building the related tuple slot received from the provider.
---
 src/backend/replication/logical/relation.c | 57 ++++++++++++++++------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index b386f8460d..cfc34d49e0 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -220,6 +220,8 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 {
 	LogicalRepRelMapEntry *entry;
 	bool		found;
+	Oid		localreloid = InvalidOid;
+	LogicalRepRelation	*remoterel;
 
 	if (LogicalRepRelMap == NULL)
 		logicalrep_relmap_init();
@@ -232,29 +234,56 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 		elog(ERROR, "no relation map entry for remote relation ID %u",
 			 remoteid);
 
-	/* Need to update the local cache? */
+	remoterel = &entry->remoterel;
+
+	/*
+	 * entry->localreloid is set to InvalidOid when the local relation is
+	 * either unknown or invalidated.
+	 * Moreover, when opening and locking a relation, pending invalidation
+	 * messages are processed.
+	 * Because of that, we first need to open the local relation, then rebuild
+	 * the mapping of local attribute if the relation is invalidated.
+	 */
+
+	/*
+	 * Unknown or invalidated local relation. First look for its oid and open
+	 * it. We will build the mapping of local attributes after.
+	 */
 	if (!OidIsValid(entry->localreloid))
 	{
 		Oid			relid;
-		int			i;
-		int			found;
-		Bitmapset  *idkey;
-		TupleDesc	desc;
-		LogicalRepRelation *remoterel;
-		MemoryContext oldctx;
-
-		remoterel = &entry->remoterel;
 
-		/* Try to find and lock the relation by name. */
+		/* Try to find the relation by name */
 		relid = RangeVarGetRelid(makeRangeVar(remoterel->nspname,
 											  remoterel->relname, -1),
-								 lockmode, true);
+								 NoLock, true);
 		if (!OidIsValid(relid))
 			ereport(ERROR,
 					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 					 errmsg("logical replication target relation \"%s.%s\" does not exist",
 							remoterel->nspname, remoterel->relname)));
-		entry->localrel = table_open(relid, NoLock);
+
+		localreloid = relid;
+	}
+	else
+		localreloid = entry->localreloid;
+
+	/* Actually open and lock the related local relation */
+	entry->localrel = table_open(localreloid, lockmode);
+
+	/*
+	 * Pending invalidation messages might have been processed while grabbing
+	 * the lock during table_open, applying some DDL operations that are then
+	 * reflected in the relcache.
+	 * Because of that, we need to rebuild the mapping of local attribute after
+	 * the relation is opened.
+	 */
+	if (!OidIsValid(entry->localreloid)) {
+		int			found;
+		Bitmapset  *idkey;
+		TupleDesc	desc;
+		MemoryContext oldctx;
+		int			i;
 
 		/* Check for supported relkind. */
 		CheckSubscriptionRelkind(entry->localrel->rd_rel->relkind,
@@ -348,10 +377,8 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 			}
 		}
 
-		entry->localreloid = relid;
+		entry->localreloid = localreloid;
 	}
-	else
-		entry->localrel = table_open(entry->localreloid, lockmode);
 
 	if (entry->state != SUBREL_STATE_READY)
 		entry->state = GetSubscriptionRelState(MySubscription->oid,
-- 
2.20.1