[Noah Misch <noah@leadboat.com>]

On Sun, Apr 07, 2024 at 01:22:51AM +0300, Alexander Korotkov wrote:
> I've pushed 0001 and 0002

The partition MERGE (1adf16b8f) and SPLIT (87c21bb94) v17 patches introduced
createPartitionTable() with this code:

	createStmt->relation = newPartName;
...
	wrapper->utilityStmt = (Node *) createStmt;
...
	ProcessUtility(wrapper,
...
	newRel = table_openrv(newPartName, NoLock);

This breaks from the CVE-2014-0062 (commit 5f17304) principle of not repeating
name lookups.  The attached demo uses this defect to make one partition have
two parents.

Attachments:

repro-merge-partition-race-v0.patch (text/plain, 6.0K) download
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ae2efdc..654b502 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3495,7 +3495,11 @@ StorePartitionBound(Relation rel, Relation parent, PartitionBoundSpec *bound)
 		elog(ERROR, "cache lookup failed for relation %u",
 			 RelationGetRelid(rel));
 
-#ifdef USE_ASSERT_CHECKING
+	/*
+	 * Assertion fails during partition getting multiple parents.  Disable the
+	 * assertion, to see what non-assert builds experience.
+	 */
+#if 0
 	{
 		Form_pg_class classForm;
 		bool		isnull;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8fcb188..48207f9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -95,6 +95,7 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
+#include "utils/injection_point.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -15825,7 +15826,11 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel, bool ispart
 			 */
 			if (parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 			{
-				Assert(child_att->attinhcount == 1);
+				/*
+				 * Assertion fails during partition getting multiple parents.
+				 * Disable, to see what non-assert builds experience.
+				 */
+				/* Assert(child_att->attinhcount == 1); */
 				child_att->attislocal = false;
 			}
 
@@ -20388,7 +20393,14 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 	 * Open the new partition with no lock, because we already have
 	 * AccessExclusiveLock placed there after creation.
 	 */
-	newRel = table_openrv(newPartName, NoLock);
+	INJECTION_POINT("merge-after-create");
+	/*
+	 * For testing, switch to taking a lock.  This solves two problems.
+	 * First, it gets an AcceptInvalidationMessages(), so we actually
+	 * invalidate the search path.  Second, it avoids an assertion failure
+	 * from our lack of lock, so we see what non-assert builds experience.
+	 */
+	newRel = table_openrv(newPartName, AccessExclusiveLock);
 
 	/*
 	 * We intended to create the partition with the same persistence as the
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 2ffd2f7..9af45bf 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -9,7 +9,7 @@ PGFILEDESC = "injection_points - facility for injection points"
 REGRESS = injection_points
 REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
 
-ISOLATION = inplace
+ISOLATION = inplace merge
 
 # The injection points are cluster-wide, so disable installcheck
 NO_INSTALLCHECK = 1
diff --git a/src/test/modules/injection_points/expected/merge.out b/src/test/modules/injection_points/expected/merge.out
new file mode 100644
index 0000000..04920e7
--- /dev/null
+++ b/src/test/modules/injection_points/expected/merge.out
@@ -0,0 +1,60 @@
+Parsed test spec with 3 sessions
+
+starting permutation: merge1 ct2 detach3
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step merge1: 
+	SET search_path = target;
+	ALTER TABLE parted MERGE PARTITIONS (part1, part3) INTO part_all;
+	-- only one of *parted should have rows, but both do
+	SELECT a AS new_target_parted FROM target.parted ORDER BY 1;
+	SELECT a AS old_target_parted FROM old_target.parted ORDER BY 1;
+	SELECT a AS new_target_part_all FROM target.part_all ORDER BY 1;
+	SELECT a AS old_target_part_all FROM old_target.part_all ORDER BY 1;
+ <waiting ...>
+step ct2: 
+	ALTER SCHEMA target RENAME TO old_target;
+	CREATE SCHEMA target
+		CREATE TABLE parted (a int) partition by list (a)
+		CREATE TABLE part_all PARTITION OF parted FOR VALUES IN (1, 2, 3, 4)
+
+step detach3: 
+	SELECT injection_points_detach('merge-after-create');
+	SELECT injection_points_wakeup('merge-after-create');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step merge1: <... completed>
+new_target_parted
+-----------------
+                1
+                3
+(2 rows)
+
+old_target_parted
+-----------------
+                1
+                3
+(2 rows)
+
+new_target_part_all
+-------------------
+                  1
+                  3
+(2 rows)
+
+old_target_part_all
+-------------------
+(0 rows)
+
diff --git a/src/test/modules/injection_points/specs/merge.spec b/src/test/modules/injection_points/specs/merge.spec
new file mode 100644
index 0000000..63bcd2b
--- /dev/null
+++ b/src/test/modules/injection_points/specs/merge.spec
@@ -0,0 +1,51 @@
+setup
+{
+	CREATE EXTENSION injection_points;
+	CREATE SCHEMA target
+		CREATE TABLE parted (a int) partition by list (a)
+		CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1, 2)
+		CREATE TABLE part3 PARTITION OF parted FOR VALUES IN (3, 4);
+	INSERT INTO target.parted VALUES (1),(3);
+}
+teardown
+{
+	DROP SCHEMA target, old_target CASCADE;
+	DROP EXTENSION injection_points;
+}
+
+# MERGE PARTITIONS
+session s1
+setup	{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('merge-after-create', 'wait');
+}
+step merge1	{
+	SET search_path = target;
+	ALTER TABLE parted MERGE PARTITIONS (part1, part3) INTO part_all;
+	-- only one of *parted should have rows, but both do
+	SELECT a AS new_target_parted FROM target.parted ORDER BY 1;
+	SELECT a AS old_target_parted FROM old_target.parted ORDER BY 1;
+	SELECT a AS new_target_part_all FROM target.part_all ORDER BY 1;
+	SELECT a AS old_target_part_all FROM old_target.part_all ORDER BY 1;
+}
+
+
+# inject another table via ALTER SCHEMA RENAME
+session s2
+step ct2	{
+	ALTER SCHEMA target RENAME TO old_target;
+	CREATE SCHEMA target
+		CREATE TABLE parted (a int) partition by list (a)
+		CREATE TABLE part_all PARTITION OF parted FOR VALUES IN (1, 2, 3, 4)
+}
+
+
+# injection release
+session s3
+step detach3	{
+	SELECT injection_points_detach('merge-after-create');
+	SELECT injection_points_wakeup('merge-after-create');
+}
+
+
+permutation merge1(detach3) ct2 detach3