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

[Andrea Bolognani]

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 at redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  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