Re: [PATCH] Fix libxml leaks in contrib/xml2 XPath functions

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