On Wed, Jul 22, 2020 at 11:39:55AM +0200, Michal Privoznik wrote:
When building domain's private /dev in a namespace, libdevmapper is consulted for getting full dependency tree of domain's disks. The reason is that for a multipath devices all dependent devices must be created in the namespace and allowed in CGroups.
However, this approach is very fragile as building of namespace happens in the forked off child process, after mass close of FDs and just before dropping privileges and execing QEMU. And it so happens that when calling libdevmapper APIs, one of them opens /dev/mapper/control and saves the FD into a global variable. The FD is kept open until the lib is unlinked or dm_lib_release() is called explicitly. We are doing neither.
This is not a problem when calling the function from libvirtd (when setting up CGroups), but it is a problem when called from the pre-exec hook because we leak the FD into QEMU.
Fixes: a30078cb832646177defd256e77c632905f1e6d0 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1858260
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virdevmapper.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c index 40a82285f9..1c216fb6c1 100644 --- a/src/util/virdevmapper.c +++ b/src/util/virdevmapper.c @@ -156,6 +156,7 @@ virDevMapperGetTargetsImpl(const char *path, virStringListFree(recursiveDevPaths); virStringListFree(devPaths); dm_task_destroy(dmt); + dm_lib_release(); return ret; }
Hmm, this function isn't threadsafe though, so I'm kind of worried about us breaking parallel callers. For that matter dm_task_run isn't thread safe when it opens the FD in the first place either AFAICT. This libdevice-mapper.so looks like a bit of a disaster in general, as I can't see mutexes acquired anywhere in the public APIs and it has a load of static global variables including this FD :-( We could put mutex locking around our own virDevMapperGetTargetsImpl if we blindly assume nothing else we call may secretly be using libdevice-mapper too. Makes me wonder though if there's any way we can obtain the info we need without touching libdevice-mapper.so at all. 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 :|