agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feed[PATCH] xml2: Fix stylesheet document leak in xslt_process()
2+ messages / 2 participants
[nested] [flat]
* [PATCH] xml2: Fix stylesheet document leak in xslt_process()
@ 2026-06-04 23:46 Andrey Chernyy <andrey.cherny@tantorlabs.com>
0 siblings, 1 reply; 2+ messages in thread
From: Andrey Chernyy @ 2026-06-04 23:46 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hi,
While following up on the recent xml2 XPath leak fixes at
https://postgr.es/m/20260601010124.5edf9a20@andrnote, I noticed the same
class of libxml/libxslt ownership issue in xslt_process().
xslt_process() parses the stylesheet argument with xmlReadMemory(), then
passes the resulting xmlDoc to xsltParseStylesheetDoc(). On failure,
libxslt leaves that document owned by the caller, as can be seen from
its own xsltParseStylesheetFile() wrapper. Postgres currently cannot
release it in the error cleanup path because ssdoc is scoped inside the
PG_TRY block.
The attached patch keeps ssdoc visible to the cleanup path, clears it
once ownership has been transferred to the stylesheet, and frees it in
PG_CATCH if parsing failed before that transfer completed.
I also attached a manual repro script. It repeatedly calls
xslt_process() with a large XML document that is not a stylesheet,
catches the ERROR in one backend, and samples VmRSS via /proc.
On my machine, without the patch, the same backend kept growing after
each failed parse:
NOTICE: xslt_process i=1, failures=1, total_kb=34668, diff_kb=12992
NOTICE: xslt_process i=2, failures=1, total_kb=44428, diff_kb=9760
NOTICE: xslt_process i=3, failures=1, total_kb=54120, diff_kb=9692
NOTICE: xslt_process i=4, failures=1, total_kb=63808, diff_kb=9688
NOTICE: xslt_process i=5, failures=1, total_kb=73496, diff_kb=9688
NOTICE: xslt_process i=6, failures=1, total_kb=83188, diff_kb=9692
NOTICE: xslt_process i=7, failures=1, total_kb=92876, diff_kb=9688
NOTICE: xslt_process i=8, failures=1, total_kb=102564, diff_kb=9688
NOTICE: xslt_process i=9, failures=1, total_kb=112256, diff_kb=9692
NOTICE: xslt_process i=10, failures=1, total_kb=121944, diff_kb=9688
With the patch, it plateaued after the initial warmup:
NOTICE: xslt_process i=1, failures=1, total_kb=23228, diff_kb=1596
NOTICE: xslt_process i=2, failures=1, total_kb=23888, diff_kb=660
NOTICE: xslt_process i=3, failures=1, total_kb=23888, diff_kb=0
NOTICE: xslt_process i=4, failures=1, total_kb=23888, diff_kb=0
NOTICE: xslt_process i=5, failures=1, total_kb=23888, diff_kb=0
NOTICE: xslt_process i=6, failures=1, total_kb=23888, diff_kb=0
NOTICE: xslt_process i=7, failures=1, total_kb=23888, diff_kb=0
NOTICE: xslt_process i=8, failures=1, total_kb=23888, diff_kb=0
NOTICE: xslt_process i=9, failures=1, total_kb=23888, diff_kb=0
NOTICE: xslt_process i=10, failures=1, total_kb=23888, diff_kb=0
--
Andrey Chernyy
Attachments:
[application/sql] xml2-xslt-process-leak-repro.sql (1.5K, ../../20260605024642.5a1b6518@andrnote/2-xml2-xslt-process-leak-repro.sql)
download
[text/x-patch] 0001-xml2-Fix-stylesheet-document-leak-in-xslt_process.patch (2.2K, ../../20260605024642.5a1b6518@andrnote/3-0001-xml2-Fix-stylesheet-document-leak-in-xslt_process.patch)
download | inline diff:
From c183f9b1d0d1bceb2e57db64ee928d7697b175d4 Mon Sep 17 00:00:00 2001
From: Andrey Chernyy <andrey.cherny@tantorlabs.com>
Date: Fri, 5 Jun 2026 02:43:39 +0300
Subject: [PATCH] xml2: Fix stylesheet document leak in xslt_process()
xslt_process() parses the stylesheet text into an xmlDoc before passing it
to xsltParseStylesheetDoc(). On success, the returned stylesheet owns
that document and frees it through xsltFreeStylesheet().
On failure, libxslt leaves the caller responsible for the xmlDoc, as
shown by its own xsltParseStylesheetFile() wrapper. Keep tracking the
stylesheet document until ownership has been transferred so the error
cleanup path can free it.
---
contrib/xml2/xslt_proc.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c
index 8ceb8c46494..b83d28fe579 100644
--- a/contrib/xml2/xslt_proc.c
+++ b/contrib/xml2/xslt_proc.c
@@ -55,6 +55,7 @@ xslt_process(PG_FUNCTION_ARGS)
PgXmlErrorContext *xmlerrcxt;
volatile xsltStylesheetPtr stylesheet = NULL;
volatile xmlDocPtr doctree = NULL;
+ volatile xmlDocPtr ssdoc = NULL;
volatile xmlDocPtr restree = NULL;
volatile xsltSecurityPrefsPtr xslt_sec_prefs = NULL;
volatile xsltTransformContextPtr xslt_ctxt = NULL;
@@ -78,7 +79,6 @@ xslt_process(PG_FUNCTION_ARGS)
PG_TRY();
{
- xmlDocPtr ssdoc;
bool xslt_sec_prefs_error;
int reslen = 0;
@@ -100,8 +100,13 @@ xslt_process(PG_FUNCTION_ARGS)
xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
"error parsing stylesheet as XML document");
- /* After this call we need not free ssdoc separately */
+ /*
+ * On success, the stylesheet owns ssdoc. On failure, libxslt leaves
+ * the caller responsible for freeing ssdoc.
+ */
stylesheet = xsltParseStylesheetDoc(ssdoc);
+ if (stylesheet != NULL)
+ ssdoc = NULL;
if (stylesheet == NULL || pg_xml_error_occurred(xmlerrcxt))
xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
@@ -167,6 +172,8 @@ xslt_process(PG_FUNCTION_ARGS)
xsltFreeSecurityPrefs(xslt_sec_prefs);
if (stylesheet != NULL)
xsltFreeStylesheet(stylesheet);
+ if (ssdoc != NULL)
+ xmlFreeDoc(ssdoc);
if (doctree != NULL)
xmlFreeDoc(doctree);
if (resstr != NULL)
--
2.54.0
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [PATCH] xml2: Fix stylesheet document leak in xslt_process()
@ 2026-06-05 04:53 Michael Paquier <michael@paquier.xyz>
parent: Andrey Chernyy <andrey.cherny@tantorlabs.com>
0 siblings, 0 replies; 2+ messages in thread
From: Michael Paquier @ 2026-06-05 04:53 UTC (permalink / raw)
To: Andrey Chernyy <andrey.cherny@tantorlabs.com>; +Cc: pgsql-hackers@lists.postgresql.org
On Fri, Jun 05, 2026 at 02:46:42AM +0300, Andrey Chernyy wrote:
> xslt_process() parses the stylesheet argument with xmlReadMemory(), then
> passes the resulting xmlDoc to xsltParseStylesheetDoc(). On failure,
> libxslt leaves that document owned by the caller, as can be seen from
> its own xsltParseStylesheetFile() wrapper. Postgres currently cannot
> release it in the error cleanup path because ssdoc is scoped inside the
> PG_TRY block.
In libxslt/xslt.c, the top comment of xsltParseStylesheetDoc() says:
"the doc is automatically freed when the stylesheet is closed."
Reading the code, I can confirm that xsltFreeStylesheet() does a bunch
of stuff, and that it has the idea to call xmlFreeDoc() once at the
end.
Will address this one as well. Thanks.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aiJWUH85UH4nVex_@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-06-05 04:53 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-04 23:46 [PATCH] xml2: Fix stylesheet document leak in xslt_process() Andrey Chernyy <andrey.cherny@tantorlabs.com>
2026-06-05 04:53 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox