Hi Michael, I checked v2, and the two additional error-path cases make sense to me.
While looking at pgxml_xpath(), I noticed one more nearby cleanup gap. pgxml_xpath() builds an xpath_workspace before returning it to callers. The callers already wrap pgxml_xpath() with PG_TRY/PG_CATCH, but if an ERROR is thrown before pgxml_xpath() returns, the assignment of the workspace pointer in the caller has not completed yet. The caller-side PG_CATCH blocks therefore cannot call cleanup_workspace() for the partially-built workspace. Attached is an incremental patch on top of your v2-0001..v2-0003. It adds local cleanup in pgxml_xpath(), tracks comppath for the error path, and checks xmlXPathNewContext() before dereferencing the returned context. I kept it separate for review, but it can of course be folded if that makes the final series cleaner. I also used the attached manual repro script, which catches repeated XPath syntax errors in a single backend and samples VmRSS from /proc. On my checkout without this pgxml_xpath() cleanup it kept growing after each round, for example: postgres=# \i xml2-pgxml-xpath-error-leak-repro.sql NOTICE: error i=1, failures=20, total_kb=109748, diff_kb=88008 NOTICE: error i=2, failures=20, total_kb=193016, diff_kb=83268 NOTICE: error i=3, failures=20, total_kb=276220, diff_kb=83204 NOTICE: error i=4, failures=20, total_kb=359428, diff_kb=83208 NOTICE: error i=5, failures=20, total_kb=442632, diff_kb=83204 NOTICE: error i=6, failures=20, total_kb=525840, diff_kb=83208 NOTICE: error i=7, failures=20, total_kb=609044, diff_kb=83204 NOTICE: error i=8, failures=20, total_kb=692252, diff_kb=83208 NOTICE: error i=9, failures=20, total_kb=775456, diff_kb=83204 NOTICE: error i=10, failures=20, total_kb=858664, diff_kb=83208 With this patch applied, it plateaued after the initial warmup: postgres=# \i xml2-pgxml-xpath-error-leak-repro.sql NOTICE: error i=1, failures=20, total_kb=22672, diff_kb=972 NOTICE: error i=2, failures=20, total_kb=22692, diff_kb=20 NOTICE: error i=3, failures=20, total_kb=22704, diff_kb=8 NOTICE: error i=4, failures=20, total_kb=22700, diff_kb=-8 NOTICE: error i=5, failures=20, total_kb=22704, diff_kb=4 NOTICE: error i=6, failures=20, total_kb=22704, diff_kb=0 NOTICE: error i=7, failures=20, total_kb=22708, diff_kb=4 NOTICE: error i=8, failures=20, total_kb=22704, diff_kb=-8 NOTICE: error i=9, failures=20, total_kb=22708, diff_kb=4 NOTICE: error i=10, failures=20, total_kb=22708, diff_kb=-4 -- Andrey Chernyy
xml2-pgxml-xpath-error-leak-repro.sql
Description: application/sql
>From fbe84d97d2256f6289d75b37735f2bbd4bc6c853 Mon Sep 17 00:00:00 2001 From: Andrey Chernyy <[email protected]> Date: Tue, 2 Jun 2026 01:41:38 +0300 Subject: [PATCH v2 4/4] xml2: Avoid libxml leaks in pgxml_xpath() error paths pgxml_xpath() builds an xpath_workspace before returning it to callers. If an ERROR is thrown before the function returns, callers have not received the workspace pointer yet and cannot run cleanup_workspace(). Add local error cleanup for the partially built workspace and the compiled XPath expression. Also check xmlXPathNewContext() before dereferencing the returned context. --- contrib/xml2/xpath.c | 51 +++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c index 9fe75cb5ff4..283bb51178d 100644 --- a/contrib/xml2/xpath.c +++ b/contrib/xml2/xpath.c @@ -497,36 +497,49 @@ static xpath_workspace * pgxml_xpath(text *document, xmlChar *xpath, PgXmlErrorContext *xmlerrcxt) { int32 docsize = VARSIZE_ANY_EXHDR(document); - xmlXPathCompExprPtr comppath; + xmlXPathCompExprPtr volatile comppath = NULL; xpath_workspace *workspace = palloc0_object(xpath_workspace); workspace->doctree = NULL; workspace->ctxt = NULL; workspace->res = NULL; - workspace->doctree = xmlReadMemory((char *) VARDATA_ANY(document), - docsize, NULL, NULL, - XML_PARSE_NOENT); - if (workspace->doctree != NULL) + PG_TRY(); { - workspace->ctxt = xmlXPathNewContext(workspace->doctree); - workspace->ctxt->node = xmlDocGetRootElement(workspace->doctree); - - /* compile the path */ - comppath = xmlXPathCtxtCompile(workspace->ctxt, xpath); - if (comppath == NULL || pg_xml_error_occurred(xmlerrcxt)) + workspace->doctree = xmlReadMemory((char *) VARDATA_ANY(document), + docsize, NULL, NULL, + XML_PARSE_NOENT); + if (workspace->doctree != NULL) { - if (comppath != NULL) - xmlXPathFreeCompExpr(comppath); - xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY, - "XPath Syntax Error"); - } + workspace->ctxt = xmlXPathNewContext(workspace->doctree); + if (workspace->ctxt == NULL) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate XPath context"); - /* Now evaluate the path expression. */ - workspace->res = xmlXPathCompiledEval(comppath, workspace->ctxt); + workspace->ctxt->node = xmlDocGetRootElement(workspace->doctree); + + /* compile the path */ + comppath = xmlXPathCtxtCompile(workspace->ctxt, xpath); + if (comppath == NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY, + "XPath Syntax Error"); + + /* Now evaluate the path expression. */ + workspace->res = xmlXPathCompiledEval(comppath, workspace->ctxt); + + xmlXPathFreeCompExpr(comppath); + comppath = NULL; + } + } + PG_CATCH(); + { + if (comppath != NULL) + xmlXPathFreeCompExpr(comppath); + cleanup_workspace(workspace); - xmlXPathFreeCompExpr(comppath); + PG_RE_THROW(); } + PG_END_TRY(); return workspace; } -- 2.54.0
