[Pkg-libvirt-commits] [libvirt] 03/04: Fix crashes in lxcDomain{S, G}etMemoryParameters

Guido Guenther agx at moszumanska.debian.org
Fri Dec 20 08:24:46 UTC 2013


This is an automated email from the git hooks/post-receive script.

agx pushed a commit to annotated tag debian/1.2.0-1
in repository libvirt.

commit e9de7b2133fc0c62b5004573abc12f32e362f5bd
Author: Guido Günther <agx at sigxcpu.org>
Date:   Wed Dec 18 08:15:27 2013 +0100

    Fix crashes in lxcDomain{S,G}etMemoryParameters
    
    This fixes CVE-2013-6436
---
 ...fix-crash-in-lxcDomainGetMemoryParameters.patch | 146 +++++++++++++++
 ...fix-crash-in-lxcDomainSetMemoryParameters.patch | 200 +++++++++++++++++++++
 debian/patches/series                              |   2 +
 3 files changed, 348 insertions(+)

diff --git a/debian/patches/security/security-fix-crash-in-lxcDomainGetMemoryParameters.patch b/debian/patches/security/security-fix-crash-in-lxcDomainGetMemoryParameters.patch
new file mode 100644
index 0000000..45c9f5a
--- /dev/null
+++ b/debian/patches/security/security-fix-crash-in-lxcDomainGetMemoryParameters.patch
@@ -0,0 +1,146 @@
+From: Martin Kletzander <mkletzan at redhat.com>
+Date: Mon, 9 Dec 2013 11:15:11 +0100
+Subject: security: fix crash in lxcDomainGetMemoryParameters
+
+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 at 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 61a90ca..477d30f 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;
+ }
+ 
diff --git a/debian/patches/security/security-fix-crash-in-lxcDomainSetMemoryParameters.patch b/debian/patches/security/security-fix-crash-in-lxcDomainSetMemoryParameters.patch
new file mode 100644
index 0000000..385938e
--- /dev/null
+++ b/debian/patches/security/security-fix-crash-in-lxcDomainSetMemoryParameters.patch
@@ -0,0 +1,200 @@
+From: Martin Kletzander <mkletzan at redhat.com>
+Date: Mon, 9 Dec 2013 11:15:12 +0100
+Subject: security: fix crash in lxcDomainSetMemoryParameters
+
+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> --hard-limit 133T'
+
+Backtrace:
+ Thread 6 (Thread 0x7fffec8c0700 (LWP 26826)):
+ #0  0x00007ffff70edcc4 in virCgroupPathOfController (group=0x0, controller=3,
+     key=0x7ffff75734bd "memory.limit_in_bytes", path=0x7fffec8bf718) at util/vircgroup.c:1764
+ #1  0x00007ffff70e9206 in virCgroupSetValueStr (group=0x0, controller=3,
+     key=0x7ffff75734bd "memory.limit_in_bytes", value=0x7fffe409f360 "1073741824")
+     at util/vircgroup.c:669
+ #2  0x00007ffff70e98b4 in virCgroupSetValueU64 (group=0x0, controller=3,
+     key=0x7ffff75734bd "memory.limit_in_bytes", value=1073741824) at util/vircgroup.c:740
+ #3  0x00007ffff70ee518 in virCgroupSetMemory (group=0x0, kb=1048576) at util/vircgroup.c:1904
+ #4  0x00007ffff70ee675 in virCgroupSetMemoryHardLimit (group=0x0, kb=1048576)
+     at util/vircgroup.c:1944
+ #5  0x00005555557d54c8 in lxcDomainSetMemoryParameters (dom=0x7fffe40cc420,
+     params=0x7fffe409f100, nparams=1, flags=0) at lxc/lxc_driver.c:774
+ #6  0x00007ffff72c20f9 in virDomainSetMemoryParameters (domain=0x7fffe40cc420,
+     params=0x7fffe409f100, nparams=1, flags=0) at libvirt.c:4051
+ #7  0x000055555561365f in remoteDispatchDomainSetMemoryParameters (server=0x555555eb7e00,
+     client=0x555555ec4b10, msg=0x555555eb94e0, rerr=0x7fffec8bfb70, args=0x7fffe40b8510)
+     at remote_dispatch.h:7621
+ #8  0x00005555556133fd in remoteDispatchDomainSetMemoryParametersHelper (server=0x555555eb7e00,
+     client=0x555555ec4b10, msg=0x555555eb94e0, rerr=0x7fffec8bfb70, args=0x7fffe40b8510,
+     ret=0x7fffe40b84f0) at remote_dispatch.h:7591
+ #9  0x00007ffff73b293f in virNetServerProgramDispatchCall (prog=0x555555ec3ae0,
+     server=0x555555eb7e00, client=0x555555ec4b10, msg=0x555555eb94e0)
+     at rpc/virnetserverprogram.c:435
+ #10 0x00007ffff73b207f in virNetServerProgramDispatch (prog=0x555555ec3ae0,
+     server=0x555555eb7e00, client=0x555555ec4b10, msg=0x555555eb94e0)
+     at rpc/virnetserverprogram.c:305
+ #11 0x00007ffff73a4d2c in virNetServerProcessMsg (srv=0x555555eb7e00, client=0x555555ec4b10,
+     prog=0x555555ec3ae0, msg=0x555555eb94e0) at rpc/virnetserver.c:165
+ #12 0x00007ffff73a4e8d in virNetServerHandleJob (jobOpaque=0x555555ec3e30, opaque=0x555555eb7e00)
+     at rpc/virnetserver.c:186
+ #13 0x00007ffff7187f3f in virThreadPoolWorker (opaque=0x555555eb7ac0) at util/virthreadpool.c:144
+ #14 0x00007ffff718733a in virThreadHelper (data=0x555555eb7890) at util/virthreadpthread.c:161
+ #15 0x00007ffff468ed89 in start_thread (arg=0x7fffec8c0700) at pthread_create.c:308
+ #16 0x00007ffff3da26bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
+
+Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
+---
+ src/lxc/lxc_driver.c | 112 +++++++++++++++++++++++++++++++++++++++++++--------
+ 1 file changed, 96 insertions(+), 16 deletions(-)
+
+diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
+index 477d30f..fde05ec 100644
+--- a/src/lxc/lxc_driver.c
++++ b/src/lxc/lxc_driver.c
+@@ -742,12 +742,24 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
+                              int nparams,
+                              unsigned int flags)
+ {
+-    size_t i;
++    virCapsPtr caps = NULL;
++    virDomainDefPtr vmdef = NULL;
+     virDomainObjPtr vm = NULL;
++    virLXCDomainObjPrivatePtr priv = NULL;
++    virLXCDriverConfigPtr cfg = NULL;
++    virLXCDriverPtr driver = dom->conn->privateData;
++    unsigned long long hard_limit;
++    unsigned long long soft_limit;
++    unsigned long long swap_hard_limit;
++    bool set_hard_limit = false;
++    bool set_soft_limit = false;
++    bool set_swap_hard_limit = false;
++    int rc;
+     int ret = -1;
+-    virLXCDomainObjPrivatePtr priv;
+ 
+-    virCheckFlags(0, -1);
++    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
++                  VIR_DOMAIN_AFFECT_CONFIG, -1);
++
+     if (virTypedParamsValidate(params, nparams,
+                                VIR_DOMAIN_MEMORY_HARD_LIMIT,
+                                VIR_TYPED_PARAM_ULLONG,
+@@ -762,29 +774,97 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
+         goto cleanup;
+ 
+     priv = vm->privateData;
++    cfg = virLXCDriverGetConfig(driver);
+ 
+-    if (virDomainSetMemoryParametersEnsureACL(dom->conn, vm->def, flags) < 0)
++    if (virDomainSetMemoryParametersEnsureACL(dom->conn, vm->def, flags) < 0 ||
++        !(caps = virLXCDriverGetCapabilities(driver, false)) ||
++        virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
++                                        vm, &flags, &vmdef) < 0)
+         goto cleanup;
+ 
+-    ret = 0;
+-    for (i = 0; i < nparams; i++) {
+-        virTypedParameterPtr param = &params[i];
++    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;
++    }
++
++#define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE)                                \
++    if ((rc = virTypedParamsGetULLong(params, nparams, PARAM, &VALUE)) < 0)  \
++        goto cleanup;                                                        \
++                                                                             \
++    if (rc == 1)                                                             \
++        set_ ## VALUE = true;
++
++    VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit)
++    VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit)
++    VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit)
++
++#undef VIR_GET_LIMIT_PARAMETER
++
++    /* Swap hard limit must be greater than hard limit.
++     * Note that limit of 0 denotes unlimited */
++    if (set_swap_hard_limit || set_hard_limit) {
++        unsigned long long mem_limit = vm->def->mem.hard_limit;
++        unsigned long long swap_limit = vm->def->mem.swap_hard_limit;
++
++        if (set_swap_hard_limit)
++            swap_limit = swap_hard_limit;
+ 
+-        if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) {
+-            if (virCgroupSetMemoryHardLimit(priv->cgroup, params[i].value.ul) < 0)
+-                ret = -1;
+-        } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) {
+-            if (virCgroupSetMemorySoftLimit(priv->cgroup, params[i].value.ul) < 0)
+-                ret = -1;
+-        } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) {
+-            if (virCgroupSetMemSwapHardLimit(priv->cgroup, params[i].value.ul) < 0)
+-                ret = -1;
++        if (set_hard_limit)
++            mem_limit = hard_limit;
++
++        if (virCompareLimitUlong(mem_limit, swap_limit) > 0) {
++            virReportError(VIR_ERR_INVALID_ARG, "%s",
++                           _("memory hard_limit tunable value must be lower "
++                             "than or equal to swap_hard_limit"));
++            goto cleanup;
+         }
+     }
+ 
++#define LXC_SET_MEM_PARAMETER(FUNC, VALUE)                                     \
++    if (set_ ## VALUE) {                                                        \
++        if (flags & VIR_DOMAIN_AFFECT_LIVE) {                                   \
++            if ((rc = FUNC(priv->cgroup, VALUE)) < 0) {                         \
++                virReportSystemError(-rc, _("unable to set memory %s tunable"), \
++                                     #VALUE);                                   \
++                                                                                \
++                goto cleanup;                                                   \
++            }                                                                   \
++            vm->def->mem.VALUE = VALUE;                                         \
++        }                                                                       \
++                                                                                \
++        if (flags & VIR_DOMAIN_AFFECT_CONFIG)                                   \
++            vmdef->mem.VALUE = VALUE;                                   \
++    }
++
++    /* Soft limit doesn't clash with the others */
++    LXC_SET_MEM_PARAMETER(virCgroupSetMemorySoftLimit, soft_limit);
++
++    /* set hard limit before swap hard limit if decreasing it */
++    if (virCompareLimitUlong(vm->def->mem.hard_limit, hard_limit) > 0) {
++        LXC_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit);
++        /* inhibit changing the limit a second time */
++        set_hard_limit = false;
++    }
++
++    LXC_SET_MEM_PARAMETER(virCgroupSetMemSwapHardLimit, swap_hard_limit);
++
++    /* otherwise increase it after swap hard limit */
++    LXC_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit);
++
++#undef LXC_SET_MEM_PARAMETER
++
++    if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
++        virDomainSaveConfig(cfg->configDir, vmdef) < 0)
++        goto cleanup;
++
++    ret = 0;
+ cleanup:
+     if (vm)
+         virObjectUnlock(vm);
++    virObjectUnref(caps);
++    virObjectUnref(cfg);
+     return ret;
+ }
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 6148614..6a8e304 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -10,3 +10,5 @@ Don-t-fail-if-we-can-t-setup-avahi.patch
 Reduce-udevadm-settle-timeout-to-10-seconds.patch
 debian/Debianize-systemd-service-files.patch
 Allow-xen-toolstack-to-find-it-s-binaries.patch
+security/security-fix-crash-in-lxcDomainGetMemoryParameters.patch
+security/security-fix-crash-in-lxcDomainSetMemoryParameters.patch

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-libvirt/libvirt.git



More information about the Pkg-libvirt-commits mailing list