From 3844d7b908db0b0df4ffda978c323bf3ac2f63c5 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))