From c6d47123ce2de0ae10ce95b717463a7490b0f9e7 Mon Sep 17 00:00:00 2001

Message-Id: <c6d47123ce2de0ae10ce95b717463a7490b0f9e7.1390394206.git.jdenemar@redhat.com>

From: Martin Kletzander <mkletzan@redhat.com>

Date: Mon, 13 Jan 2014 12:09:21 +0100

Subject: [PATCH] CVE-2013-6436: fix crash in lxcDomainGetMemoryParameters

https://bugzilla.redhat.com/show_bug.cgi?id=1049137

The function doesn't check whether the request is made for active or

inactive domain.  Thus when the domain is not running it still tries

accessing non-existing cgroups (priv->cgroup, which is NULL).

I re-made the function in order for it to work the same way it's qemu

counterpart does.

Reproducer:

 1) Define an LXC domain

 2) Do 'virsh memtune <domain>'

Backtrace:

 Thread 6 (Thread 0x7fffec8c0700 (LWP 13387)):

 #0  0x00007ffff70edcc4 in virCgroupPathOfController (group=0x0, controller=3,

     key=0x7ffff75734bd "memory.limit_in_bytes", path=0x7fffec8bf750) at util/vircgroup.c:1764

 #1  0x00007ffff70e958c in virCgroupGetValueStr (group=0x0, controller=3,

     key=0x7ffff75734bd "memory.limit_in_bytes", value=0x7fffec8bf7c0) at util/vircgroup.c:705

 #2  0x00007ffff70e9d29 in virCgroupGetValueU64 (group=0x0, controller=3,

     key=0x7ffff75734bd "memory.limit_in_bytes", value=0x7fffec8bf810) at util/vircgroup.c:804

 #3  0x00007ffff70ee706 in virCgroupGetMemoryHardLimit (group=0x0, kb=0x7fffec8bf8a8)

     at util/vircgroup.c:1962

 #4  0x00005555557d590f in lxcDomainGetMemoryParameters (dom=0x7fffd40024a0,

     params=0x7fffd40027a0, nparams=0x7fffec8bfa24, flags=0) at lxc/lxc_driver.c:826

 #5  0x00007ffff72c28d3 in virDomainGetMemoryParameters (domain=0x7fffd40024a0,

     params=0x7fffd40027a0, nparams=0x7fffec8bfa24, flags=0) at libvirt.c:4137

 #6  0x000055555563714d in remoteDispatchDomainGetMemoryParameters (server=0x555555eb7e00,

     client=0x555555ebaef0, msg=0x555555ebb3e0, rerr=0x7fffec8bfb70, args=0x7fffd40024e0,

     ret=0x7fffd4002420) at remote.c:1895

 #7  0x00005555556052c4 in remoteDispatchDomainGetMemoryParametersHelper (server=0x555555eb7e00,

     client=0x555555ebaef0, msg=0x555555ebb3e0, rerr=0x7fffec8bfb70, args=0x7fffd40024e0,

     ret=0x7fffd4002420) at remote_dispatch.h:4050

 #8  0x00007ffff73b293f in virNetServerProgramDispatchCall (prog=0x555555ec3ae0,

     server=0x555555eb7e00, client=0x555555ebaef0, msg=0x555555ebb3e0)

     at rpc/virnetserverprogram.c:435

 #9  0x00007ffff73b207f in virNetServerProgramDispatch (prog=0x555555ec3ae0,

     server=0x555555eb7e00, client=0x555555ebaef0, msg=0x555555ebb3e0)

     at rpc/virnetserverprogram.c:305

 #10 0x00007ffff73a4d2c in virNetServerProcessMsg (srv=0x555555eb7e00, client=0x555555ebaef0,

     prog=0x555555ec3ae0, msg=0x555555ebb3e0) at rpc/virnetserver.c:165

 #11 0x00007ffff73a4e8d in virNetServerHandleJob (jobOpaque=0x555555ebc7e0, opaque=0x555555eb7e00)

     at rpc/virnetserver.c:186

 #12 0x00007ffff7187f3f in virThreadPoolWorker (opaque=0x555555eb7ac0) at util/virthreadpool.c:144

 #13 0x00007ffff718733a in virThreadHelper (data=0x555555eb7890) at util/virthreadpthread.c:161

 #14 0x00007ffff468ed89 in start_thread (arg=0x7fffec8c0700) at pthread_create.c:308

 #15 0x00007ffff3da26bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>

(cherry picked from commit f8c1cb90213508c4f32549023b0572ed774e48aa)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/lxc/lxc_driver.c | 41 ++++++++++++++++++++++++++++++++++-------

 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c

index b746ffd..90d4878 100644

--- a/src/lxc/lxc_driver.c

+++ b/src/lxc/lxc_driver.c

@@ -794,22 +794,36 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,

                              int *nparams,

                              unsigned int flags)

 {

-    size_t i;

+    virCapsPtr caps = NULL;

+    virDomainDefPtr vmdef = NULL;

     virDomainObjPtr vm = NULL;

+    virLXCDomainObjPrivatePtr priv = NULL;

+    virLXCDriverPtr driver = dom->conn->privateData;

     unsigned long long val;

     int ret = -1;

-    virLXCDomainObjPrivatePtr priv;

+    size_t i;

-    virCheckFlags(0, -1);

+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |

+                  VIR_DOMAIN_AFFECT_CONFIG, -1);

     if (!(vm = lxcDomObjFromDomain(dom)))

         goto cleanup;

     priv = vm->privateData;

-    if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0)

+    if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0 ||

+        !(caps = virLXCDriverGetCapabilities(driver, false)) ||

+        virDomainLiveConfigHelperMethod(caps, driver->xmlopt,

+                                        vm, &flags, &vmdef) < 0)

         goto cleanup;

+    if (flags & VIR_DOMAIN_AFFECT_LIVE &&

+        !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {

+        virReportError(VIR_ERR_OPERATION_INVALID,

+                       "%s", _("cgroup memory controller is not mounted"));

+        goto cleanup;

+    }

+

     if ((*nparams) == 0) {

         /* Current number of memory parameters supported by cgroups */

         *nparams = LXC_NB_MEM_PARAM;

@@ -823,22 +837,34 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,

         switch (i) {

         case 0: /* fill memory hard limit here */

-            if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0)

+            if (flags & VIR_DOMAIN_AFFECT_CONFIG) {

+                val = vmdef->mem.hard_limit;

+                val = val ? val : VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;

+            } else if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0) {

                 goto cleanup;

+            }

             if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_HARD_LIMIT,

                                         VIR_TYPED_PARAM_ULLONG, val) < 0)

                 goto cleanup;

             break;

         case 1: /* fill memory soft limit here */

-            if (virCgroupGetMemorySoftLimit(priv->cgroup, &val) < 0)

+            if (flags & VIR_DOMAIN_AFFECT_CONFIG) {

+                val = vmdef->mem.soft_limit;

+                val = val ? val : VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;

+            } else if (virCgroupGetMemorySoftLimit(priv->cgroup, &val) < 0) {

                 goto cleanup;

+            }

             if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_SOFT_LIMIT,

                                         VIR_TYPED_PARAM_ULLONG, val) < 0)

                 goto cleanup;

             break;

         case 2: /* fill swap hard limit here */

-            if (virCgroupGetMemSwapHardLimit(priv->cgroup, &val) < 0)

+            if (flags & VIR_DOMAIN_AFFECT_CONFIG) {

+                val = vmdef->mem.swap_hard_limit;

+                val = val ? val : VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;

+            } else if (virCgroupGetMemSwapHardLimit(priv->cgroup, &val) < 0) {

                 goto cleanup;

+            }

             if (virTypedParameterAssign(param,

                                         VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,

                                         VIR_TYPED_PARAM_ULLONG, val) < 0)

@@ -859,6 +885,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,

 cleanup:

     if (vm)

         virObjectUnlock(vm);

+    virObjectUnref(caps);

     return ret;

 }

-- 

1.8.5.3