Re: [PATCH 3/4] virdevmapper: Don't use libdevmapper to obtain dependencies

On Mon, 2020-07-27 at 10:31 +0200, Michal Privoznik wrote: > CVE-2020-14339 > > 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. > > However, the virDevMapperGetTargets() function is called also > from libvirtd (when setting up CGroups) and thus has to be thread > safe. Unfortunately, libdevmapper APIs are not thread safe (nor > async signal safe) and thus we can't use them. Reimplement what > libdevmapper would do using plain C (ioctl()-s, /proc/devices > parsing, /dev/mapper dirwalking, and so on). > > Fixes: a30078cb832646177defd256e77c632905f1e6d0 > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1858260 > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > po/POTFILES.in | 1 + > src/util/virdevmapper.c | 304 ++++++++++++++++++++++++++++------------ > 2 files changed, 219 insertions(+), 86 deletions(-) This patch broke libvirt in Debian for certain setups. With AppArmor enabled (the default), the error is $ virsh start cirros error: Failed to start domain cirros error: internal error: Process exited prior to exec: libvirt: error : Cannot delete directory '/run/libvirt/qemu/1-cirros.dev': Device or resource busy If I disable AppArmor by passing security='' on the kernel command line, the error message changes to $ virsh start cirros error: Failed to start domain cirros error: internal error: Process exited prior to exec: libvirt: QEMU Driver error : Unable to get devmapper targets for /var/lib/libvirt/images/cirros.qcow2: Success An effective workaround is to set namespaces=[] in qemu.conf, but that's of course not something that we want users doing :) The underlying issue seems to be caused by the fact that, on a Debian installation that uses plain partitions instead of LVM, /proc/devices doesn't contain an entry for device-mapper right after boot, which... > static int > virDevMapperOnceInit(void) > { > - /* Ideally, we would not need this. But libdevmapper prints > - * error messages to stderr by default. Sad but true. */ > - dm_log_with_errno_init(virDevMapperDummyLogger); > + g_autofree char *buf = NULL; > + VIR_AUTOSTRINGLIST lines = NULL; > + size_t i; > + > + if (virFileReadAll(PROC_DEVICES, BUF_SIZE, &buf) < 0) > + return -1; > + > + lines = virStringSplit(buf, "\n", 0); > + if (!lines) > + return -1; > + > + for (i = 0; lines[i]; i++) { > + g_autofree char *dev = NULL; > + unsigned int maj; > + > + if (sscanf(lines[i], "%u %ms\n", &maj, &dev) == 2 && > + STREQ(dev, DM_NAME)) { > + virDMMajor = maj; > + break; > + } > + } > + > + if (!lines[i]) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unable to find major for %s"), > + DM_NAME); > + return -1; > + } ... this code expects. Running $ sudo dmsetup info No devices found

causes the entry to appear, and from that moment on guest startup will work as expected regardless of whether or not AppArmor is enabled. I hope the information above can help someone who's familiar with the code figure out a fix. I'll provide more if needed, just ask! I can also provide prebuilt .deb files for 6.6.0 that consistently trigger the issue when added to a bog standard Debian testing installation. -- Andrea Bolognani / Red Hat / Virtualization