On Sat, Feb 10, 2018 at 07:06:22AM +0100, Michal Privoznik wrote:
> On 02/07/2018 02:13 PM, Daniel P. Berrangé wrote:
> > On Wed, Feb 07, 2018 at 09:58:21AM +0530, P J P wrote:
> >> +-- On Mon, 5 Feb 2018, Daniel P. Berrangé wrote --+
> >> | From: Lubomir Rintel <lkund...@v3.sk>
> >> | 
> >> | At later point it might not be possible or even safe to use 
> >> getaddrinfo(). It
> >> | can in turn result in a load of NSS module.
> >> | 
> >> | Notably, on a LXC container startup we may find ourselves with the guest
> >> | filesystem already having replaced the host one. Loading a NSS module
> >> | from the guest tree could allow a malicous guest to escape the
> >> | confinement of its container environment because libvirt will not yet
> >> | have locked it down.
> >> | ---
> >> | 
> >> | NB, we're still awaiting CVE allocation before pushing to git
> >>
> >> 'CVE-2018-6764' has been assigned to this issue by Mitre.
> > 
> > Thanks, I have pushed this patch now
> > 
> 
> Actually, I think we need to revisit this patch. It makes
> gethostbyname() deadlock if libvirt NSS plugin is enabled. Here's the
> stack trace of what's going on (I've ran `getent hosts libvirt.org`):
> 
> #0  0x00007f6e714b57e7 in futex_wait (private=0, expected=1, 
> futex_word=0x7f6e71f2fb80 <virLogOnceControl>) at 
> ../sysdeps/unix/sysv/linux/futex-internal.h:61
> #1  futex_wait_simple (private=0, expected=1, futex_word=0x7f6e71f2fb80 
> <virLogOnceControl>) at ../sysdeps/nptl/futex-internal.h:135
> #2  __pthread_once_slow (once_control=0x7f6e71f2fb80 <virLogOnceControl>, 
> init_routine=0x7f6e71d09949 <virLogOnce>) at pthread_once.c:105
> #3  0x00007f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 <virLogOnceControl>, 
> init=0x7f6e71d09949 <virLogOnce>) at util/virthread.c:48
> #4  0x00007f6e71d0997c in virLogInitialize () at util/virlog.c:285
> #5  0x00007f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 <virLogSelf>, 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
>     fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s", 
> vargs=0x7ffe45fab6e0) at util/virlog.c:582
> #6  0x00007f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 <virLogSelf>, 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
>     fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s") at util/virlog.c:542
> #7  0x00007f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at 
> util/virobject.c:254
> #8  0x00007f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at 
> util/virobject.c:272
> #9  0x00007f6e71d0d3e5 in virMacMapNew (file=0x56348c520e70 
> "/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319
> #10 0x00007f6e71cdc50a in findLease (name=0x7ffe45fac1d0 "burns", af=0, 
> address=0x7ffe45fab980, naddress=0x7ffe45fab988, found=0x7ffe45fab973, 
> errnop=0x7ffe45fabb88) at nss/libvirt_nss.c:301
> #11 0x00007f6e71cdcc56 in _nss_libvirt_gethostbyname4_r (name=0x7ffe45fac1d0 
> "burns", pat=0x7ffe45fabb98, buffer=0x7ffe45fabd10 "& ", buflen=1024, 
> errnop=0x7ffe45fabb88, herrnop=0x7ffe45fabbb0, ttlp=0x0) at 
> nss/libvirt_nss.c:522
> #12 0x00007f6e724631fc in gaih_inet (name=name@entry=0x7ffe45fac1d0 "burns", 
> service=<optimized out>, req=req@entry=0x7ffe45fac1a0, 
> pai=pai@entry=0x7ffe45fabc98, naddrs=naddrs@entry=0x7ffe45fabc94, 
> tmpbuf=tmpbuf@entry=0x7ffe45fabd00)
>     at ../sysdeps/posix/getaddrinfo.c:825
> #13 0x00007f6e72464697 in __GI_getaddrinfo (name=<optimized out>, 
> service=<optimized out>, hints=0x7ffe45fac1a0, pai=0x7ffe45fac198) at 
> ../sysdeps/posix/getaddrinfo.c:2370
> #14 0x00007f6e71d19e81 in virGetHostnameImpl (quiet=true) at 
> util/virutil.c:717
> #15 0x00007f6e71d1a057 in virGetHostnameQuiet () at util/virutil.c:759
> #16 0x00007f6e71d09936 in virLogOnceInit () at util/virlog.c:279
> #17 0x00007f6e71d09952 in virLogOnce () at util/virlog.c:285
> #18 0x00007f6e714b5829 in __pthread_once_slow (once_control=0x7f6e71f2fb80 
> <virLogOnceControl>, init_routine=0x7f6e71d09949 <virLogOnce>) at 
> pthread_once.c:116
> #19 0x00007f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 <virLogOnceControl>, 
> init=0x7f6e71d09949 <virLogOnce>) at util/virthread.c:48
> #20 0x00007f6e71d0997c in virLogInitialize () at util/virlog.c:285
> #21 0x00007f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 <virLogSelf>, 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
>     fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s", 
> vargs=0x7ffe45fac410) at util/virlog.c:582
> #22 0x00007f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 <virLogSelf>, 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
>     fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s") at util/virlog.c:542
> #23 0x00007f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at 
> util/virobject.c:254
> #24 0x00007f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at 
> util/virobject.c:272
> #25 0x00007f6e71d0d3e5 in virMacMapNew (file=0x56348c51c430 
> "/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319
> #26 0x00007f6e71cdc50a in findLease (name=0x7ffe45faddc5 "libvirt.org", 
> af=10, address=0x7ffe45fac6c0, naddress=0x7ffe45fac6c8, found=0x7ffe45fac6b3, 
> errnop=0x7f6e7290a698) at nss/libvirt_nss.c:301
> #27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r (name=0x7ffe45faddc5 
> "libvirt.org", af=10, result=0x7f6e7272f200 <resbuf>, buffer=0x56348c511000 
> "& ", buflen=1024, errnop=0x7f6e7290a698, herrnop=0x7ffe45fac85c, ttlp=0x0, 
> canonp=0x0)
>     at nss/libvirt_nss.c:412
> #28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r (name=0x7ffe45faddc5 
> "libvirt.org", af=10, result=0x7f6e7272f200 <resbuf>, buffer=0x56348c511000 
> "& ", buflen=1024, errnop=0x7f6e7290a698, herrnop=0x7ffe45fac85c) at 
> nss/libvirt_nss.c:372
> #29 0x00007f6e7248f72f in __gethostbyname2_r (name=name@entry=0x7ffe45faddc5 
> "libvirt.org", af=af@entry=10, resbuf=resbuf@entry=0x7f6e7272f200 <resbuf>, 
> buffer=<optimized out>, buflen=1024, result=result@entry=0x7ffe45fac860, 
> h_errnop=0x7ffe45fac85c)
>     at ../nss/getXXbyYY_r.c:314
> #30 0x00007f6e7248f494 in gethostbyname2 (name=0x7ffe45faddc5 "libvirt.org", 
> af=10) at ../nss/getXXbyYY.c:116
> #31 0x000056348c30c36d in hosts_keys (number=<optimized out>, key=<optimized 
> out>) at getent.c:317
> #32 0x000056348c30b7d2 in main (argc=<optimized out>, argv=0x7ffe45faca28) at 
> getent.c:955
> 
> So in frames 28 an 27 libvirt nss module is consulted. However, this
> means that virLog*() is issued from one of many internal APIs the NSS
> plugin uses. So it causes virLogInitialize() to be called. However,
> after this patch was pushed in we try to resolve the hostname in
> virGetHostnameImpl() which leads to NSS plugin and thus completes the
> circle and deadlocks.
> 
> I'm not quite sure how to solve this issue. Ideally, the NSS plugin
> wouldn't use logging at all (and it isn't directly just indirectly) but
> is there a way to not compile in virlog.c? We can turn off DEBUG but
> that's not going to be enough (as you can see in the stack trace).

I think we should just call hostname()  directly in virLogInitialize.
The other stuff virGetHostname() does is not really relevant for the
usage we have in logging.

Also, the virLogOnce call ought to be done outside the logging lock,
since the pthread once stuff already guarantees serialization and
single execution without needing the caller to provide a mutex.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to