[libvirt] [PATCH 0/2] Fix crash handling memory parameters in LXC
The first of these 2 issues is a security issue CVE-2013-6436 Both patches have been pushed to GIT master. Martin Kletzander (2): CVE-2013-6436: fix crash in lxcDomainGetMemoryParameters Fix crash in lxcDomainSetMemoryParameters src/lxc/lxc_driver.c | 153 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 130 insertions(+), 23 deletions(-) -- 1.8.4.2
From: Martin Kletzander <mkletzan@redhat.com> 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> --- 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 c499182..16258cd 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.4.2
From: Martin Kletzander <mkletzan@redhat.com> 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@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 16258cd..e5298d1 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 = ¶ms[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; } -- 1.8.4.2
participants (1)
-
Daniel P. Berrange