On Tue, Aug 04, 2020 at 11:39 PM +0200, Andrea Bolognani <abologna@xxxxxxxxxx> wrote: > 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@xxxxxxxxxx> >> Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> >> --- >> 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 We see the same problem as mentioned by Andrea. The host kernel configuration used: … CONFIG_BLK_DEV_DM_BUILTIN=y CONFIG_BLK_DEV_DM=m … As soon as we load the kernel module ‘dm-mod‘ everything works because then ‘device-mapper‘ is listed in /proc/devices. > > 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 > -- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294