agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Andrey Chernyy <andrey.cherny@tantorlabs.com>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Fix libxml leaks in contrib/xml2 XPath functions
Date: Tue, 2 Jun 2026 03:43:14 +0300
Message-ID: <20260602034314.7eadc4d7@andrnote> (raw)
In-Reply-To: <ah0E0xLvv5_OtOi6@paquier.xyz>
References: <20260601010124.5edf9a20@andrnote>
<ah0E0xLvv5_OtOi6@paquier.xyz>
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
Attachments:
[application/sql] xml2-pgxml-xpath-error-leak-repro.sql (1.4K, ../20260602034314.7eadc4d7@andrnote/2-xml2-pgxml-xpath-error-leak-repro.sql)
download
[text/x-patch] v2-0004-xml2-Avoid-libxml-leaks-in-pgxml_xpath-error-path.patch (3.1K, ../20260602034314.7eadc4d7@andrnote/3-v2-0004-xml2-Avoid-libxml-leaks-in-pgxml_xpath-error-path.patch)
download | inline diff:
From fbe84d97d2256f6289d75b37735f2bbd4bc6c853 Mon Sep 17 00:00:00 2001
From: Andrey Chernyy <andrey.cherny@tantorlabs.com>
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
view thread (5+ messages) latest in thread
Message-ID: <20260602034314.7eadc4d7@andrnote>
Permalink: ../20260602034314.7eadc4d7@andrnote/
Also on: postgresql.org/message-id/20260602034314.7eadc4d7@andrnote
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: andrey.cherny@tantorlabs.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Fix libxml leaks in contrib/xml2 XPath functions
In-Reply-To: <20260602034314.7eadc4d7@andrnote>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox