[PATCH] Fix libxml leaks in contrib/xml2 XPath functions
Hi,
While reviewing contrib/xml2 I found two successful-path libxml leaks in
XPath functions.
The attached series is:
0001 Fix libxml string leak in contrib/xml2 xpath_list
0002 Fix libxml leaks in contrib/xml2 xpath_table
In xpath_list(), the plain separator path passes the result of
xmlXPathCastNodeToString() directly to xmlBufferWriteCHAR().
xmlXPathCastNodeToString() returns a libxml-allocated xmlChar * that
has to be released with xmlFree(), while xmlBufferWriteCHAR() copies
the string rather than taking ownership.
In xpath_table(), xmlXPathCompiledEval() returns an xmlXPathObjectPtr
that has to be released with xmlXPathFreeObject(). The function also
stores libxml-allocated xmlChar strings in the values array before
calling BuildTupleFromCStrings(). BuildTupleFromCStrings() consumes
those strings by converting/copying them into the result tuple, so the
temporary libxml strings can be released after the tuple is built.
The second patch frees the XPath result object after each evaluation,
frees per-column string values after BuildTupleFromCStrings() has
consumed them, and tracks the current libxml allocations across the
existing PG_TRY block so they are also released on error.
The attached manual repro scripts exercise the two paths separately.
They are Linux-specific because they sample VmRSS from
/proc/<backend-pid>/status using pg_read_file(), so they should be run
as a superuser or a role allowed to read server files.
On unpatched origin/master, selected NOTICE lines from the repro scripts
show steady backend RSS growth:
postgres=# \i ./xml2-xpath-list-leak-repro.sql
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=1,
total_kb=38616, diff_kb=17220 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=2, total_kb=40732, diff_kb=2104
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=3,
total_kb=42720, diff_kb=1988 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=4, total_kb=44644, diff_kb=1924
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=5,
total_kb=46440, diff_kb=1796 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=6, total_kb=47968, diff_kb=1528
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=7,
total_kb=50892, diff_kb=2924 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=8, total_kb=52712, diff_kb=1820
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=9,
total_kb=54276, diff_kb=1564 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=10, total_kb=55328, diff_kb=1052
postgres=# \i ./xml2-xpath-table-leak-repro.sql
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=1,
total_kb=26452, diff_kb=5136 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=2, total_kb=27772, diff_kb=1312
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=3,
total_kb=29068, diff_kb=1296 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=4, total_kb=30368, diff_kb=1300
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=5,
total_kb=31668, diff_kb=1300 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=6, total_kb=32968, diff_kb=1300
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=7,
total_kb=34260, diff_kb=1292 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=8, total_kb=35568, diff_kb=1308
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=9,
total_kb=36876, diff_kb=1308 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=10, total_kb=38168, diff_kb=1292
With both patches applied, the same scripts plateau after the initial
allocations:
postgres=# \i ./xml2-xpath-list-leak-repro.sql
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=1,
total_kb=24480, diff_kb=3020 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=2, total_kb=24532, diff_kb=44
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=3,
total_kb=24580, diff_kb=48 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=4, total_kb=24580, diff_kb=0
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=5,
total_kb=24624, diff_kb=44 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=6, total_kb=24580, diff_kb=-44
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=7,
total_kb=24580, diff_kb=0 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=8, total_kb=24580, diff_kb=0
psql:xml2-xpath-list-leak-repro.sql:38: NOTICE: xpath_list i=9,
total_kb=24580, diff_kb=0 psql:xml2-xpath-list-leak-repro.sql:38:
NOTICE: xpath_list i=10, total_kb=24580, diff_kb=0
postgres=# \i ./xml2-xpath-table-leak-repro.sql
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=1,
total_kb=26572, diff_kb=188 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=2, total_kb=26600, diff_kb=28
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=3,
total_kb=26600, diff_kb=0 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=4, total_kb=26600, diff_kb=0
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=5,
total_kb=26600, diff_kb=0 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=6, total_kb=26600, diff_kb=0
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=7,
total_kb=26600, diff_kb=0 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=8, total_kb=26600, diff_kb=0
psql:xml2-xpath-table-leak-repro.sql:40: NOTICE: xpath_table i=9,
total_kb=26600, diff_kb=0 psql:xml2-xpath-table-leak-repro.sql:40:
NOTICE: xpath_table i=10, total_kb=26600, diff_kb=0
contrib/xml2 regression tests still pass:
make -C contrib/xml2 check
# All 1 tests passed.
I also checked that the two patches apply cleanly to current
origin/master with git am.
This is related to, but not fixed by, the recent xml2/libxml
error-handling work from BUG #18943 / commit 732061150b0. That patch
improved cleanup and OOM handling around libxml calls, while these
cases are successful execution path leaks.
These are long-standing leaks and look like candidates for
back-patching to all supported branches.
--
Andrey Chernyy
On Mon, Jun 01, 2026 at 01:01:24AM +0300, Andrey Chernyy wrote:
This is related to, but not fixed by, the recent xml2/libxml
error-handling work from BUG #18943 / commit 732061150b0. That patch
improved cleanup and OOM handling around libxml calls, while these
cases are successful execution path leaks.These are long-standing leaks and look like candidates for
back-patching to all supported branches.
Right. I have missed that xmlXPathCastNodeToString() does an
allocation of the result, and it is documented in the upstream code
that the caller is responsible for freeing what's been returned.
+ xmlXPathContextPtr volatile ctxt = NULL;
+ xmlXPathObjectPtr volatile res = NULL;
+ xmlXPathCompExprPtr volatile comppath = NULL;
+ xmlChar *volatile resstr = NULL;For these fout, ctxt is reachable once xmlXPathNewContext() does not
return NULL. For res, that's xmlXPathCompiledEval, and caller has to
free the result (documented). comppath is limited to the case where
the call of xmlXPathCtxtCompile() allocates a result, returns non-NULL
and has an error, which is a short window, still reachable. resstr
can be reached under when pg_xml_error_occurred() fails with an
allocation done.
I can see two more problematic patterns, that happen in error-only
paths still are worth addressing:
- pgxmlNodeSetToText() is missing a xmlFree() for "result".
- xmlXPathCtxtCompile() in pgxml_xpath(), where pg_xml_error_occurred
is reached and an allocation is done.
732061150b0 was only done on HEAD because these leaks were considered
as not worth the trouble in the back branches. This could qualify as
an open item, even if the leaks you are pointing at here are much
older than that.
Attached is a patch for the code paths I have grabbed on the way.
I'll take care of that, merging everything together. Thanks for the
report!
--
Michael