[libvirt] [PATCH RFC 6/7] qemu: Spawn qemu under mount namespace

[Michal Privoznik]

On 14.11.2016 17:55, Daniel P. Berrange wrote:
> On Mon, Nov 14, 2016 at 05:43:30PM +0100, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/qemu/qemu_domain.c  | 233 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  src/qemu/qemu_domain.h  |   8 ++
>>  src/qemu/qemu_process.c |  13 +++
>>  3 files changed, 254 insertions(+)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 8cba755..3a0170c 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -55,6 +55,7 @@
>>  
>>  #include <sys/time.h>
>>  #include <fcntl.h>
>> +#include <sys/mount.h>
>>  
>>  #include <libxml/xpathInternals.h>
>>  
>> @@ -86,6 +87,21 @@ VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST,
>>                "start",
>>  );
>>  
>> +#define QEMU_DEV_MAJ_MEMORY  1
>> +#define QEMU_DEV_MAJ_TTY     5
>> +#define QEMU_DEV_MAJ_KVM     10
>> +#define QEMU_DEV_MAJ_PTY     136
>> +
>> +#define QEMU_DEV_MIN_CONSOLE 1
>> +#define QEMU_DEV_MIN_FULL    7
>> +#define QEMU_DEV_MIN_FUSE    229
>> +#define QEMU_DEV_MIN_KVM     232
>> +#define QEMU_DEV_MIN_NULL    3
>> +#define QEMU_DEV_MIN_PTMX    2
>> +#define QEMU_DEV_MIN_RANDOM  8
>> +#define QEMU_DEV_MIN_TTY     0
>> +#define QEMU_DEV_MIN_URANDOM 9
>> +#define QEMU_DEV_MIN_ZERO    5
>>  
>>  struct _qemuDomainLogContext {
>>      int refs;
>> @@ -6658,3 +6674,220 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
>>  
>>      return true;
>>  }
>> +
>> +
>> +static int
>> +qemuDomainPopulateDevices(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>> +                          virDomainObjPtr vm ATTRIBUTE_UNUSED,
>> +                          const char *path)
>> +{
>> +    int ret = -1;
>> +    virFileDevices devs[] = {
>> +        { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_NULL, 0666, "/null" },
>> +        { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_ZERO, 0666, "/zero" },
>> +        { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_FULL, 0666, "/full" },
>> +        { QEMU_DEV_MAJ_KVM,  QEMU_DEV_MIN_KVM, 0660, "/kvm"},
>> +        { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_RANDOM, 0666, "/random" },
>> +        { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_URANDOM, 0666, "/urandom" },
>> +        { QEMU_DEV_MAJ_TTY, QEMU_DEV_MIN_TTY, 0666, "/tty" },
>> +        { 0, 0, 0, NULL},
>> +    };
>> +
>> +    if (virFilePopulateDevices(path, devs) < 0)
>> +        goto cleanup;
>> +
>> +    ret = 0;
>> + cleanup:
>> +    return ret;
>> +}
>> +
>> +
>> +static int
>> +qemuDomainSetupDev(virQEMUDriverPtr driver,
>> +                   virDomainObjPtr vm,
>> +                   const char *path)
>> +{
>> +    char *mount_options = NULL;
>> +    char *opts = NULL;
>> +    int ret = -1;
>> +
>> +    VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name);
>> +
>> +    mount_options = virSecurityManagerGetMountOptions(driver->securityManager,
>> +                                                      vm->def);
>> +
>> +    if (!mount_options &&
>> +        VIR_STRDUP(mount_options, "") < 0)
>> +        goto cleanup;
>> +
>> +    /*
>> +     * tmpfs is limited to 64kb, since we only have device nodes in there
>> +     * and don't want to DOS the entire OS RAM usage
>> +     */
>> +    if (virAsprintf(&opts,
>> +                    "mode=755,size=65536%s", mount_options) < 0)
>> +        goto cleanup;
>> +
>> +    if (virFileSetupDev(path, opts) < 0)
>> +        goto cleanup;
>> +
>> +    if (qemuDomainPopulateDevices(driver, vm, path) < 0)
>> +        goto cleanup;
>> +
>> +    ret = 0;
>> + cleanup:
>> +    VIR_FREE(opts);
>> +    VIR_FREE(mount_options);
>> +    return ret;
>> +}
>> +
>> +
>> +static int
>> +qemuDomainSetupDevPTS(virQEMUDriverPtr driver,
>> +                      virDomainObjPtr vm,
>> +                      const char *path)
>> +{
>> +    char *mount_options = NULL;
>> +    char *opts = NULL;
>> +    int ret = -1;
>> +    const gid_t ptsgid = 5;
>> +
>> +    mount_options = virSecurityManagerGetMountOptions(driver->securityManager,
>> +                                                      vm->def);
>> +
>> +    if (!mount_options &&
>> +        VIR_STRDUP(mount_options, "") < 0)
>> +        goto cleanup;
>> +
>> +    if (virAsprintf(&opts, "newinstance,ptmxmode=0666,mode=0620,gid=%u%s",
>> +                    ptsgid, (mount_options ? mount_options : "")) < 0)
>> +        goto cleanup;
>> +
>> +    if (virFileSetupDevPTS(path, opts, NULL) < 0)
>> +        goto cleanup;
>> +
>> +    ret = 0;
>> + cleanup:
>> +    VIR_FREE(opts);
>> +    VIR_FREE(mount_options);
>> +    return ret;
>> +}
> 
> This is going to cause problems - the /dev/pts/NNN nodes that
> QEMU has open (for its chardev backend) are exposed in the
> guest XML and are intended to be accessed by other processes
> on the host. By setting up a new /dev/pts mount with newinstance,
> the /dev/pts/NNN nodes for chardevs will be invisible to the host
> OS.
> 
> So while you want to replace /dev completely, you need to keep
> the original /dev/pts mount unchanged. Practically speaking
> this means that after doing the unshare() you need to use
> mount() with MS_MOVE to move /dev/pts somewhere safe, then
> unmount the old /dev, mount our new /dev instance and then
> finally MS_MOVE the saved /dev/pts back in place.

Ah, good point. I will fix this.

Thanks for such quick review.

Michal