Hello Peter!

I've found (using valgrind) some cases of reading random garbage after
allocated memory. Investigation showed this was caused by converting
some nodes to VciScanState* even if they have smaller size allocated
originally. So accessing VciScanState fields was accessing memory after
palloc'ed memory which could be used by any other allocation.

I think converting to VciScanState* is only valid for nodes with tag 
T_CustomScanState so here is a patch that adds assertions for this:
0001-Assert-corrrect-node-tags-when-casting-to-VciScanSta.patch

VCI v23 passes the tests with this patch applied.

There are queries that fail unfortunately. I've added one of them to
bugs.sql:
0002-Reproducer-of-invalid-cast-to-VciScanState.patch
Node with tag 420 (T_NestLoopState) is cast to VciScanState* that fails
newly added assertion.

I'm not sure where to look next to fix this. Looking forward for you
comments and ideas.

-- 
Regards,
Timur Magomedov

From 76d1d41047c06116ddc560088458d15c966868dc Mon Sep 17 00:00:00 2001
From: Timur Magomedov <[email protected]>
Date: Wed, 17 Sep 2025 17:15:00 +0300
Subject: [PATCH 1/2] Assert corrrect node tags when casting to VciScanState

---
 contrib/vci/executor/vci_agg.c          |  6 ++++++
 contrib/vci/executor/vci_plan.c         |  3 +++
 contrib/vci/executor/vci_scan.c         | 12 ++++++++++++
 contrib/vci/executor/vci_sort.c         |  4 +++-
 contrib/vci/include/vci_aggref_impl.inc |  1 +
 5 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/contrib/vci/executor/vci_agg.c b/contrib/vci/executor/vci_agg.c
index 705610b58a3..be24bb0980e 100644
--- a/contrib/vci/executor/vci_agg.c
+++ b/contrib/vci/executor/vci_agg.c
@@ -527,6 +527,8 @@ lookup_hash_entry_vector(VciAggState *aggstate,
 	uint16	   *skip_list;
 	int			slot_index;
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 	skip_list = vci_CSGetSkipFromVirtualTuples(scanstate->vector_set);
 
 	/* Clear the tuple */
@@ -786,6 +788,8 @@ agg_fill_hash_table_vector(VciAggState *aggstate)
 	ExprContext *tmpcontext;
 	VciScanState *scanstate = (VciScanState *) outerPlanState(aggstate);
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 	/*
 	 * get state info from node
 	 */
@@ -1306,6 +1310,8 @@ vci_agg_BeginCustomPlan(CustomScanState *node, EState *estate, int eflags)
 		{
 			VciScanState *scanstate = (VciScanState *) outerPlanState(aggstate);
 
+			Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 			aggstate->hash_input_values = palloc(sizeof(Datum *) * aggstate->num_hash_needed);
 			aggstate->hash_input_isnull = palloc(sizeof(bool *) * aggstate->num_hash_needed);
 
diff --git a/contrib/vci/executor/vci_plan.c b/contrib/vci/executor/vci_plan.c
index c6146789c3a..dcc78802655 100644
--- a/contrib/vci/executor/vci_plan.c
+++ b/contrib/vci/executor/vci_plan.c
@@ -175,7 +175,10 @@ static VciScanState *
 search_scan_state(PlanState *node, Plan *target)
 {
 	if (node->plan == target)
+	{
+		Assert(node->type == T_CustomScanState);
 		return (VciScanState *) node;
+	}
 
 	if (outerPlanState(node))
 	{
diff --git a/contrib/vci/executor/vci_scan.c b/contrib/vci/executor/vci_scan.c
index fbf9ce5ffc5..158c6c58e22 100644
--- a/contrib/vci/executor/vci_scan.c
+++ b/contrib/vci/executor/vci_scan.c
@@ -97,6 +97,8 @@ vci_scan_BeginCustomPlan(CustomScanState *node, EState *estate, int eflags)
 	vci_initexpr_t initexpr = VCI_INIT_EXPR_NONE;
 	TupleDesc	scanDesc;
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 	if (ScanDirectionIsBackward(estate->es_direction))
 		elog(ERROR, "VCI Scan does not support backward scan");
 
@@ -237,6 +239,8 @@ vci_scan_ExecCustomPlan(CustomScanState *cstate)
 {
 	VciScanState *scanstate = (VciScanState *) cstate;
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 	return VciExecProcScanTuple(scanstate);
 }
 
@@ -457,6 +461,8 @@ vci_scan_EndCustomPlan(CustomScanState *node)
 	VciScanState *scanstate = (VciScanState *) node;
 	TableScanDesc scanDesc;
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 	scan = (VciScan *) scanstate->vci.css.ss.ps.plan;
 
 	scanDesc = scanstate->vci.css.ss.ss_currentScanDesc;
@@ -486,6 +492,7 @@ vci_scan_ReScanCustomPlan(CustomScanState *node)
 
 	scanstate = (VciScanState *) node;
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
 	/* Rescan EvalPlanQual tuple if we're inside an EvalPlanQual recheck */
 	Assert(scanstate->vci.css.ss.ps.state->es_epq_active == NULL);
 
@@ -497,6 +504,8 @@ vci_scan_MarkPosCustomPlan(CustomScanState *node)
 {
 	VciScanState *scanstate = (VciScanState *) node;
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 	vci_mark_pos_vector_set_from_column_store(scanstate);
 }
 
@@ -505,6 +514,8 @@ vci_scan_RestrPosCustomPlan(CustomScanState *node)
 {
 	VciScanState *scanstate = (VciScanState *) node;
 
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+
 	ExecClearTuple(scanstate->vci.css.ss.ss_ScanTupleSlot);
 
 	vci_restr_pos_vector_set_from_column_store(scanstate);
@@ -523,6 +534,7 @@ vci_scan_ExplainCustomPlanTargetRel(CustomScanState *node, ExplainState *es)
 	RangeTblEntry *rte;
 
 	scanstate = (VciScanState *) node;
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
 	scan = (VciScan *) scanstate->vci.css.ss.ps.plan;
 	scanrelid = scan->scanrelid;
 
diff --git a/contrib/vci/executor/vci_sort.c b/contrib/vci/executor/vci_sort.c
index 2e91762ad7a..029e02739ab 100644
--- a/contrib/vci/executor/vci_sort.c
+++ b/contrib/vci/executor/vci_sort.c
@@ -85,7 +85,9 @@ vci_sort_ExecCustomPlan(CustomScanState *node)
 
 		for (;;)
 		{
-			slot = VciExecProcScanTuple((VciScanState *) outerNode);
+			VciScanState *scanstate = (VciScanState *) outerNode;
+			Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
+			slot = VciExecProcScanTuple(scanstate);
 
 			if (TupIsNull(slot))
 				break;
diff --git a/contrib/vci/include/vci_aggref_impl.inc b/contrib/vci/include/vci_aggref_impl.inc
index 18a11b0bf66..95fba4d19ee 100644
--- a/contrib/vci/include/vci_aggref_impl.inc
+++ b/contrib/vci/include/vci_aggref_impl.inc
@@ -62,6 +62,7 @@ VCI_ADVANCE_AGGREF_FUNC(VciAggState *aggstate,
 #endif							/* SELECT VCI_TRANS_INPUTS_ARG */
 
 	scanstate = (VciScanState *) outerPlanState(aggstate);
+	Assert(scanstate->vci.css.ss.ps.type == T_CustomScanState);
 	skip_list = vci_CSGetSkipFromVirtualTuples(scanstate->vector_set);
 
 #if VCI_TRANS_INPUTS_ARG == VCI_TRANS_INPUTS_0
-- 
2.43.0

From 6e0ab587c083f4061a85a448472d0ca258d73018 Mon Sep 17 00:00:00 2001
From: Timur Magomedov <[email protected]>
Date: Wed, 17 Sep 2025 17:59:05 +0300
Subject: [PATCH 2/2] Reproducer of invalid cast to VciScanState

---
 contrib/vci/sql/bugs.sql | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/contrib/vci/sql/bugs.sql b/contrib/vci/sql/bugs.sql
index 3b6b7303acb..49597abcf99 100644
--- a/contrib/vci/sql/bugs.sql
+++ b/contrib/vci/sql/bugs.sql
@@ -57,3 +57,23 @@ INSERT INTO t6 SELECT id, 'info' || id FROM generate_series(1, 500) id;
 ANALYZE t6;
 EXPLAIN (ANALYZE, COSTS FALSE, BUFFERS FALSE, TIMING FALSE, SUMMARY FALSE) SELECT max(id) FROM t6;
 DROP TABLE t6;
+
+CREATE TABLE main (id BIGSERIAL PRIMARY KEY);
+CREATE TABLE secondary (id BIGSERIAL PRIMARY KEY, main_id BIGINT REFERENCES main (id), val INTEGER);
+
+CREATE INDEX main_vci ON main USING vci (id);
+CREATE INDEX sec_vci ON secondary USING vci (id, main_id, val);
+
+EXPLAIN (ANALYZE, COSTS FALSE, BUFFERS FALSE, TIMING FALSE, SUMMARY FALSE)
+SELECT *
+  FROM main m
+  JOIN secondary s
+	ON m.id = s.main_id
+ WHERE s.val in (
+		SELECT MAX(val)
+		  FROM secondary s2
+		 WHERE s2.main_id = m.id)
+ ORDER BY s.val;
+
+DROP TABLE secondary;
+DROP TABLE main;
-- 
2.43.0

Reply via email to