fix crash of plpgsql_check due access to invalid memory when Cursor l…

…eaks detection is processed. Cursor leaks detection uses memory from Top Transaction Context. Unfortunately, this memory can be invalidated inside function execution (when transaction is ended inside procedure). Originaly pointer to transaction context was stored as plugin2 info. After fix, the plugin2 info is pointer to fn_mcxt context structure with pointer to transaction memory and transaction id. Before any access to transaction memory, the transaction id is checked.
okbob · Mar 29, 2024 · 9fca7e8 · 9fca7e8
1 parent 639bcbf
commit 9fca7e8
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 11 deletions.
diff --git a/src/cursors_leaks.c b/src/cursors_leaks.c
@@ -46,6 +46,12 @@ typedef struct
 	CursorTrace *cursors_traces;
 } FunctionTrace;
 
+typedef struct
+{
+	FunctionTrace *ftrace;
+	LocalTransactionId lxid;
+} CursorLeaksPlugin2Info;
+
 MemoryContextCallback contextCallback;
 
 static LocalTransactionId traces_lxid = InvalidLocalTransactionId;
@@ -121,13 +127,18 @@ get_function_trace(PLpgSQL_function *func)
 static void
 func_setup(PLpgSQL_execstate *estate, PLpgSQL_function *func, void **plugin2_info)
 {
-	FunctionTrace *ftrace;
-
 	if (plpgsql_check_cursors_leaks)
 	{
-		ftrace = get_function_trace(func);
+		CursorLeaksPlugin2Info *pinfo;
+		MemoryContext fn_mcxt;
+
+		fn_mcxt = plpgsql_check_get_current_fn_mcxt();
+		pinfo = MemoryContextAlloc(fn_mcxt, sizeof(CursorLeaksPlugin2Info));
 
-		*plugin2_info = ftrace;
+		pinfo->ftrace = get_function_trace(func);
+		pinfo->lxid = CURRENT_LXID;
+
+		*plugin2_info = pinfo;
 	}
 	else
 		*plugin2_info = NULL;
@@ -138,12 +149,15 @@ func_end(PLpgSQL_execstate *estate,
 				PLpgSQL_function *func,
 				void **plugin2_info)
 {
-	FunctionTrace *ftrace = *plugin2_info;
+	CursorLeaksPlugin2Info *pinfo = *plugin2_info;
+	FunctionTrace *ftrace;
 	int			i;
 
-	if (!ftrace || traces_lxid != CURRENT_LXID)
+	if (!pinfo || pinfo->lxid != CURRENT_LXID)
 		return;
 
+	ftrace = pinfo->ftrace;
+
 	for (i = 0; i < ftrace->ncursors; i++)
 	{
 		CursorTrace *ct = &ftrace->cursors_traces[i];
@@ -186,17 +200,23 @@ func_end(PLpgSQL_execstate *estate,
 static void
 stmt_end(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt, void **plugin2_info)
 {
-	FunctionTrace *ftrace = *plugin2_info;
+	CursorLeaksPlugin2Info *pinfo = *plugin2_info;
+	FunctionTrace *ftrace;
 
-	if (!ftrace)
+	if (!pinfo)
 		return;
 
-	if (traces_lxid != CURRENT_LXID)
+	if (traces_lxid != CURRENT_LXID ||
+		pinfo->lxid != CURRENT_LXID)
 	{
-		ftrace = get_function_trace(estate->func);
-		*plugin2_info = ftrace;
+		pinfo->ftrace = get_function_trace(estate->func);
+		pinfo->lxid = CURRENT_LXID;
+
+		*plugin2_info = pinfo;
 	}
 
+	ftrace = pinfo->ftrace;
+
 	if (stmt->cmd_type == PLPGSQL_STMT_OPEN)
 	{
 		int			i;

diff --git a/src/pldbgapi2.c b/src/pldbgapi2.c
@@ -160,6 +160,7 @@ plpgsql_check_get_current_stmts_info(void)
 	return current_fmgr_plpgsql_cache->func_info->stmts_info;
 }
 
+
 /*
  * It is used outside pldbapi2 plugins. This is used by output functions,
  * so we don't need to solve effectivity too much. Instead handling use_count
@@ -226,6 +227,16 @@ plpgsql_check_get_current_func_info_signature(void)
 	return current_fmgr_plpgsql_cache->func_info->fn_signature;
 }
 
+MemoryContext
+plpgsql_check_get_current_fn_mcxt(void)
+{
+	Assert(current_fmgr_plpgsql_cache);
+	Assert(current_fmgr_plpgsql_cache->func_info);
+	Assert(current_fmgr_plpgsql_cache->func_info->use_count > 0);
+
+	return current_fmgr_plpgsql_cache->fn_mcxt;
+}
+
 static void
 func_info_init_hashkey(func_info_hashkey *hk, PLpgSQL_function *func)
 {

diff --git a/src/plpgsql_check.h b/src/plpgsql_check.h
@@ -455,6 +455,7 @@ extern int *plpgsql_check_get_stmtid_map(PLpgSQL_function *func);
 
 extern char *plpgsql_check_get_current_func_info_name(void);
 extern char *plpgsql_check_get_current_func_info_signature(void);
+extern MemoryContext plpgsql_check_get_current_fn_mcxt(void);
 
 #if PG_VERSION_NUM < 150000