[libvirt] [PATCH] util: avoid symbol clash between json libraries

[Daniel P. Berrangé]

On Wed, Aug 01, 2018 at 09:31:49AM +0200, Peter Krempa wrote:
> On Tue, Jul 31, 2018 at 15:55:28 +0100, Daniel Berrange wrote:
> > The jansson and json-glib libraries both export symbols with a json_
> > name prefix and json_object_iter_next() clashes between them.
> > 
> > Unfortunately json_glib is linked in by GTK, so any app using GTK and
> > libvirt will get a clash, resulting in SEGV. This also affects the NSS
> > module provided by libvirt
> > 
> > Instead of directly linking to jansson, use dlopen() with the RTLD_LOCAL
> > flag which allows us to hide the symbols from the application that loads
> > libvirt or the NSS module.
> > 
> > Some preprocessor black magic and wrapper functions are used to redirect
> > calls into the dlopen resolved symbols.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> > ---
> >  libvirt.spec.in          |   2 +
> >  src/Makefile.am          |   3 +
> >  src/util/Makefile.inc.am |   3 +-
> >  src/util/virjson.c       |   9 +-
> >  src/util/virjsoncompat.c | 253 +++++++++++++++++++++++++++++++++++++++
> >  src/util/virjsoncompat.h |  86 +++++++++++++
> >  6 files changed, 354 insertions(+), 2 deletions(-)
> >  create mode 100644 src/util/virjsoncompat.c
> >  create mode 100644 src/util/virjsoncompat.h
> 
> [...]
> 
> > diff --git a/src/util/virjson.c b/src/util/virjson.c
> > index 01a387b2f7..5bab662cd3 100644
> > --- a/src/util/virjson.c
> > +++ b/src/util/virjson.c
> > @@ -1437,7 +1437,8 @@ virJSONValueCopy(const virJSONValue *in)
> >  
> >  
> >  #if WITH_JANSSON
> > -# include <jansson.h>
> > +
> > +# include "virjsoncompat.h"
> >  
> >  static virJSONValuePtr
> >  virJSONValueFromJansson(json_t *json)
> > @@ -1524,6 +1525,9 @@ virJSONValueFromString(const char *jsonstring)
> >      size_t flags = JSON_REJECT_DUPLICATES |
> >                     JSON_DECODE_ANY;
> >  
> > +    if (virJSONInitialize() < 0)
> > +        return NULL;
> > +
> >      if (!(json = json_loads(jsonstring, flags, &error))) {
> >          virReportError(VIR_ERR_INTERNAL_ERROR,
> >                         _("failed to parse JSON %d:%d: %s"),
> > @@ -1630,6 +1634,9 @@ virJSONValueToString(virJSONValuePtr object,
> >      json_t *json;
> >      char *str = NULL;
> >  
> > +    if (virJSONInitialize() < 0)
> > +        return NULL;
> > +
> >      if (pretty)
> >          flags |= JSON_INDENT(2);
> >      else
> 
> [Note that some mails did not make it to the list so I don't know if
> this issue was raised.]
> 
> If we initialize this when doing JSON operations and the initialization
> fails we'll end up with libvirtd in a crippled state not able to use
> anything in qemu which is in my opinion VERY bad.
> 
> In addition it also kills all running VMs when reconnecting since
> monitor will just not work.
> 
> I think libvirtd should terminate right away if we can't initialize this
> shim rather than execute some other code.

We can do that by calling virJSONInitialize from libvirtd main. It should
never fail unless someone instaled libvirtd without jansson being present,
or if jansson broke its API.

> Additionally, why is this even necessary for libvirtd? Not that it would
> ever link with any clashing libraries.

It may not currently be neccessary for libvirtd, but I can't guarantee
that's always going to be the case. We link to so many libraries, we can
be surprised at anytime with something pulling in a clashing json impl.
The json-c library also includes symbols with a json_* prefix in their
name, so that's a 3d json impl which might clash.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|