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

[Marc Hartmayer]

On Tue, Aug 04, 2020 at 11:39 PM +0200, Andrea Bolognani <abologna at redhat.com> 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 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

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