[libvirt] [PATCH 3/5] qemuDomainCreateDeviceRecursive: Don't try to create devices under preserved mount points

[Cedric Bosdonnat]

On Fri, 2017-04-28 at 13:22 +0200, Michal Privoznik wrote:
> While the code allows devices to already be there (by some
> miracle), we shouldn't try to create devices that don't belong to
> us. For instance, we shouldn't try to create /dev/shm/file
> because /dev/shm is a mount point that is preserved. Therefore if
> a file is created there from an outside (e.g. by mgmt application
> or some other daemon running on the system like vhostmd), it
> exists in the qemu namespace too as the mount point is the same.
> It's only /dev and /dev only that is different. The same

One 'only' should be dropped perhaps?

> reasoning applies to all other preserved mount points.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_domain.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9e18f7e..5840c57 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7339,6 +7339,8 @@ qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
>  
>  struct qemuDomainCreateDeviceData {
>      const char *path;     /* Path to temp new /dev location */
> +    char * const *devMountsPath;
> +    size_t ndevMountsPath;
>  };
>  
>  
> @@ -7392,17 +7394,34 @@ qemuDomainCreateDeviceRecursive(const char *device,
>       * For now, lets hope callers play nice.
>       */
>      if (STRPREFIX(device, DEVPREFIX)) {
> -        if (virAsprintf(&devicePath, "%s/%s",
> -                        data->path, device + strlen(DEVPREFIX)) < 0)
> -            goto cleanup;
> +        size_t i;
>  
> -        if (virFileMakeParentPath(devicePath) < 0) {
> -            virReportSystemError(errno,
> -                                 _("Unable to create %s"),
> -                                 devicePath);
> -            goto cleanup;
> +        for (i = 0; i < data->ndevMountsPath; i++) {
> +            if (STREQ(data->devMountsPath[i], "/dev"))
> +                continue;
> +            if (STRPREFIX(device, data->devMountsPath[i]))
> +                break;
> +        }
> +
> +        if (i == data->ndevMountsPath) {
> +            /* Okay, @device is in /dev but not in any mount point under /dev.
> +             * Create it. */
> +            if (virAsprintf(&devicePath, "%s/%s",
> +                            data->path, device + strlen(DEVPREFIX)) < 0)
> +                goto cleanup;
> +
> +            if (virFileMakeParentPath(devicePath) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Unable to create %s"),
> +                                     devicePath);
> +                goto cleanup;
> +            }
> +            VIR_DEBUG("Creating dev %s", device);
> +            create = true;
> +        } else {
> +            VIR_DEBUG("Skipping dev %s because of %s mount point",
> +                      device, data->devMountsPath[i]);
>          }
> -        create = true;
>      }
>  
>      if (isLink) {
> @@ -7951,6 +7970,8 @@ qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
>      }
>  
>      data.path = devPath;
> +    data.devMountsPath = devMountsPath;
> +    data.ndevMountsPath = ndevMountsPath;
>  
>      if (virProcessSetupPrivateMountNS() < 0)
>          goto cleanup;

ACK

--
Cedric