From e1e7e05376faf1ed471cb5c1d1e0415458f2af7d Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 23 Dec 2013 22:55:51 -0700
Subject: [PATCH 09/14] CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in
 LXC shutdown/reboot code

Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
lxcDomainReboot.  Otherwise, a malicious guest could use symlinks
to force the host to manipulate the wrong file in the host's namespace.

Idea by Dan Berrange, based on an initial report by Reco
<recoverym4n@gmail.com> at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=732394

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit aebbcdd33c8c18891f0bdbbf8924599a28152c9c)

Conflicts:
	src/lxc/lxc_driver.c: OOM error reporting changes
	src/util/virinitctl.c: OOM error reporting changes
---
 src/lxc/lxc_driver.c  | 42 ++++++++++++++++++++----------------------
 src/util/virinitctl.c | 30 ++++++++++--------------------
 src/util/virinitctl.h |  5 ++---
 3 files changed, 32 insertions(+), 45 deletions(-)

Index: libvirt-1.0.5.9/src/lxc/lxc_driver.c
===================================================================
--- libvirt-1.0.5.9.orig/src/lxc/lxc_driver.c
+++ libvirt-1.0.5.9/src/lxc/lxc_driver.c
@@ -2770,13 +2770,21 @@ lxcConnectListAllDomains(virConnectPtr c
 
 
 static int
+lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED,
+                         void *opaque)
+{
+    int *command = opaque;
+    return virInitctlSetRunLevel(*command);
+}
+
+
+static int
 lxcDomainShutdownFlags(virDomainPtr dom,
                        unsigned int flags)
 {
     virLXCDriverPtr driver = dom->conn->privateData;
     virLXCDomainObjPrivatePtr priv;
     virDomainObjPtr vm;
-    char *vroot = NULL;
     int ret = -1;
     int rc;
 
@@ -2809,18 +2817,14 @@ lxcDomainShutdownFlags(virDomainPtr dom,
         goto cleanup;
     }
 
-    if (virAsprintf(&vroot, "/proc/%llu/root",
-                    (unsigned long long)priv->initpid) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
     if (flags == 0 ||
         (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
-        if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
-                                        vroot)) < 0) {
+        int command = VIR_INITCTL_RUNLEVEL_POWEROFF;
+
+        if ((rc = virProcessRunInMountNamespace(priv->initpid,
+                                                lxcDomainInitctlCallback,
+                                                &command)) < 0)
             goto cleanup;
-        }
         if (rc == 0 && flags != 0 &&
             ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
             virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -2846,7 +2850,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
     ret = 0;
 
 cleanup:
-    VIR_FREE(vroot);
     if (vm)
         virObjectUnlock(vm);
     return ret;
@@ -2858,6 +2861,7 @@ lxcDomainShutdown(virDomainPtr dom)
     return lxcDomainShutdownFlags(dom, 0);
 }
 
+
 static int
 lxcDomainReboot(virDomainPtr dom,
                 unsigned int flags)
@@ -2865,7 +2869,6 @@ lxcDomainReboot(virDomainPtr dom,
     virLXCDriverPtr driver = dom->conn->privateData;
     virLXCDomainObjPrivatePtr priv;
     virDomainObjPtr vm;
-    char *vroot = NULL;
     int ret = -1;
     int rc;
 
@@ -2898,18 +2901,14 @@ lxcDomainReboot(virDomainPtr dom,
         goto cleanup;
     }
 
-    if (virAsprintf(&vroot, "/proc/%llu/root",
-                    (unsigned long long)priv->initpid) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
     if (flags == 0 ||
         (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
-        if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
-                                        vroot)) < 0) {
+        int command = VIR_INITCTL_RUNLEVEL_REBOOT;
+
+        if ((rc = virProcessRunInMountNamespace(priv->initpid,
+                                                lxcDomainInitctlCallback,
+                                                &command)) < 0)
             goto cleanup;
-        }
         if (rc == 0 && flags != 0 &&
             ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
             virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -2935,7 +2934,6 @@ lxcDomainReboot(virDomainPtr dom,
     ret = 0;
 
 cleanup:
-    VIR_FREE(vroot);
     if (vm)
         virObjectUnlock(vm);
     return ret;
Index: libvirt-1.0.5.9/src/util/virinitctl.c
===================================================================
--- libvirt-1.0.5.9.orig/src/util/virinitctl.c
+++ libvirt-1.0.5.9/src/util/virinitctl.c
@@ -110,16 +110,18 @@ struct virInitctlRequest {
 # endif
 
 /*
- * Send a message to init to change the runlevel
+ * Send a message to init to change the runlevel. This function is
+ * asynchronous-signal-safe (thus safe to use after fork of a
+ * multithreaded parent) - which is good, because it should only be
+ * used after forking and entering correct namespace.
  *
  * Returns 1 on success, 0 if initctl does not exist, -1 on error
  */
-int virInitctlSetRunLevel(virInitctlRunLevel level,
-                          const char *vroot)
+int
+virInitctlSetRunLevel(virInitctlRunLevel level)
 {
     struct virInitctlRequest req;
     int fd = -1;
-    char *path = NULL;
     int ret = -1;
 
     memset(&req, 0, sizeof(req));
@@ -130,40 +132,28 @@ int virInitctlSetRunLevel(virInitctlRunL
     /* Yes it is an 'int' field, but wants a numeric character. Go figure */
     req.runlevel = '0' + level;
 
-    if (vroot) {
-        if (virAsprintf(&path, "%s/%s", vroot, VIR_INITCTL_FIFO) < 0) {
-            virReportOOMError();
-            return -1;
-        }
-    } else {
-        if (!(path = strdup(VIR_INITCTL_FIFO))) {
-            virReportOOMError();
-            return -1;
-        }
-    }
-
-    if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) < 0) {
+    if ((fd = open(VIR_INITCTL_FIFO,
+                   O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) < 0) {
         if (errno == ENOENT) {
             ret = 0;
             goto cleanup;
         }
         virReportSystemError(errno,
                              _("Cannot open init control %s"),
-                             path);
+                             VIR_INITCTL_FIFO);
         goto cleanup;
     }
 
     if (safewrite(fd, &req, sizeof(req)) != sizeof(req)) {
         virReportSystemError(errno,
                              _("Failed to send request to init control %s"),
-                             path);
+                             VIR_INITCTL_FIFO);
         goto cleanup;
     }
 
     ret = 1;
 
 cleanup:
-    VIR_FREE(path);
     VIR_FORCE_CLOSE(fd);
     return ret;
 }
Index: libvirt-1.0.5.9/src/util/virinitctl.h
===================================================================
--- libvirt-1.0.5.9.orig/src/util/virinitctl.h
+++ libvirt-1.0.5.9/src/util/virinitctl.h
@@ -1,7 +1,7 @@
 /*
  * virinitctl.h: API for talking to init systems via initctl
  *
- * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2012-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -37,7 +37,6 @@ enum virInitctlRunLevel {
     VIR_INITCTL_RUNLEVEL_LAST
 };
 
-int virInitctlSetRunLevel(virInitctlRunLevel level,
-                          const char *vroot);
+int virInitctlSetRunLevel(virInitctlRunLevel level);
 
 #endif
