On Mon, Jun 23, 2025 at 12:02:31 +0100, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
The main XML parser code global initializer historically had a mutex protecting it, and more recently uses a pthread_once. The RelaxNG code, however, relies on three other global initializers that are not thread safe, just relying on setting an integer "initialized" flag.
Calling the relevant initializers from libvirt in a protected global initializer will protect libvirt's own concurrent usage, however, it cannot protect against other libraries loaded in process that might be using libxml2's schema code. Fortunately:
* The chances of other loaded non-libvirt code using libxml is relatively low * The chances of other loaded non-libvirt code using the schema validation / catalog functionality inside libxml is even lower * The chances of both libvirt and the non-libvirt usage having their *1st* usage of libxml2 be concurrent is tiny
Additionaly IIUC this could be problem only when using the embedded driver mode as we don't use libxml2 in the exported API
IOW, in practice, although our solution doesn't fully fix the thread safety, it is good enough.
libxml2 should none the less still be fixed to make its global initializers be thread safe without special actions by its API consumers.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/788 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/util/virxml.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/src/util/virxml.c b/src/util/virxml.c index 9d46e5f32f..c2d5a109dc 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -26,6 +26,8 @@
#include <libxml/xmlsave.h> #include <libxml/xpathInternals.h> +#include <libxml/xmlschemastypes.h> +#include <libxml/catalog.h>
#include "virerror.h" #include "virxml.h" @@ -35,6 +37,7 @@ #include "virstring.h" #include "virutil.h" #include "viruuid.h" +#include "virthread.h" #include "configmake.h"
#define VIR_FROM_THIS VIR_FROM_XML @@ -50,6 +53,25 @@ struct virParserData { };
+static int virXMLSchemaOnceInit(void)
Please use the preferred style of return type on it's own line.
+{ + if (xmlSchemaInitTypes() < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to initialize libxml2 schema types")); + return -1; + } + if (xmlRelaxNGInitTypes() < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to initialize libxml2 RelaxNG data")); + return -1; + } + /* No return value, it just ignores errors :-( */ + xmlInitializeCatalog(); + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virXMLSchema); + static xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) { @@ -1603,6 +1625,9 @@ virXMLValidatorInit(const char *schemafile) { g_autoptr(virXMLValidator) validator = NULL;
+ if (virXMLSchemaInitialize() < 0) + return NULL; + validator = g_new0(virXMLValidator, 1);
validator->schemafile = g_strdup(schemafile); -- 2.49.0
Reviewed-by: Peter Krempa <pkrempa@redhat.com>