From 2ceb47e3cd0435a1e14ae696df418283086b6f93 Mon Sep 17 00:00:00 2001 From: Hamid Akhtar Date: Tue, 18 Apr 2023 18:01:34 +0500 Subject: [PATCH] PG-613: Postgresql crashes with Segmentation fault when query plan is enabled on large queries The return value for snprintf was incorrectly being recorded as plan length. That's been resolved. As part of this fix, we've also elminated the possibility of a potential memory leak when plan text was being saved. Co-authored-by: Hamid Akhtar Co-authored-by: Muhammad Usama --- pg_stat_monitor.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/pg_stat_monitor.c b/pg_stat_monitor.c index a76a6a7..5836198 100644 --- a/pg_stat_monitor.c +++ b/pg_stat_monitor.c @@ -692,9 +692,30 @@ pgsm_ExecutorEnd(QueryDesc *queryDesc) /* Extract the plan information in case of SELECT statement */ if (queryDesc->operation == CMD_SELECT && pgsm_enable_query_plan) { - plan_info.plan_len = snprintf(plan_info.plan_text, PLAN_TEXT_LEN, "%s", pgsm_explain(queryDesc)); - plan_info.planid = pgsm_hash_string(plan_info.plan_text, plan_info.plan_len); - plan_ptr = &plan_info; + int rv; + MemoryContext oldctx; + + /* + * Making sure it is a per query context so that there's no memory + * leak when executor ends. + */ + oldctx = MemoryContextSwitchTo(queryDesc->estate->es_query_cxt); + + rv = snprintf(plan_info.plan_text, PLAN_TEXT_LEN, "%s", pgsm_explain(queryDesc)); + + /* + * If snprint didn't write anything or there was an error, let's keep + * planinfo as NULL. + */ + if (rv > 0) + { + plan_info.plan_len = (rv < PLAN_TEXT_LEN) ? rv : PLAN_TEXT_LEN - 1; + plan_info.planid = pgsm_hash_string(plan_info.plan_text, plan_info.plan_len); + plan_ptr = &plan_info; + } + + /* Switch back to old context */ + MemoryContextSwitchTo(oldctx); } if (queryId != UINT64CONST(0) && queryDesc->totaltime && pgsm_enabled(exec_nested_level))