|
0 |
From: Martin Kletzander <mkletzan@redhat.com>
|
|
1 |
Date: Mon, 9 Dec 2013 11:15:12 +0100
|
|
2 |
Subject: security: fix crash in lxcDomainSetMemoryParameters
|
|
3 |
|
|
4 |
The function doesn't check whether the request is made for active or
|
|
5 |
inactive domain. Thus when the domain is not running it still tries
|
|
6 |
accessing non-existing cgroups (priv->cgroup, which is NULL).
|
|
7 |
|
|
8 |
I re-made the function in order for it to work the same way it's qemu
|
|
9 |
counterpart does.
|
|
10 |
|
|
11 |
Reproducer:
|
|
12 |
1) Define an LXC domain
|
|
13 |
2) Do 'virsh memtune <domain> --hard-limit 133T'
|
|
14 |
|
|
15 |
Backtrace:
|
|
16 |
Thread 6 (Thread 0x7fffec8c0700 (LWP 26826)):
|
|
17 |
#0 0x00007ffff70edcc4 in virCgroupPathOfController (group=0x0, controller=3,
|
|
18 |
key=0x7ffff75734bd "memory.limit_in_bytes", path=0x7fffec8bf718) at util/vircgroup.c:1764
|
|
19 |
#1 0x00007ffff70e9206 in virCgroupSetValueStr (group=0x0, controller=3,
|
|
20 |
key=0x7ffff75734bd "memory.limit_in_bytes", value=0x7fffe409f360 "1073741824")
|
|
21 |
at util/vircgroup.c:669
|
|
22 |
#2 0x00007ffff70e98b4 in virCgroupSetValueU64 (group=0x0, controller=3,
|
|
23 |
key=0x7ffff75734bd "memory.limit_in_bytes", value=1073741824) at util/vircgroup.c:740
|
|
24 |
#3 0x00007ffff70ee518 in virCgroupSetMemory (group=0x0, kb=1048576) at util/vircgroup.c:1904
|
|
25 |
#4 0x00007ffff70ee675 in virCgroupSetMemoryHardLimit (group=0x0, kb=1048576)
|
|
26 |
at util/vircgroup.c:1944
|
|
27 |
#5 0x00005555557d54c8 in lxcDomainSetMemoryParameters (dom=0x7fffe40cc420,
|
|
28 |
params=0x7fffe409f100, nparams=1, flags=0) at lxc/lxc_driver.c:774
|
|
29 |
#6 0x00007ffff72c20f9 in virDomainSetMemoryParameters (domain=0x7fffe40cc420,
|
|
30 |
params=0x7fffe409f100, nparams=1, flags=0) at libvirt.c:4051
|
|
31 |
#7 0x000055555561365f in remoteDispatchDomainSetMemoryParameters (server=0x555555eb7e00,
|
|
32 |
client=0x555555ec4b10, msg=0x555555eb94e0, rerr=0x7fffec8bfb70, args=0x7fffe40b8510)
|
|
33 |
at remote_dispatch.h:7621
|
|
34 |
#8 0x00005555556133fd in remoteDispatchDomainSetMemoryParametersHelper (server=0x555555eb7e00,
|
|
35 |
client=0x555555ec4b10, msg=0x555555eb94e0, rerr=0x7fffec8bfb70, args=0x7fffe40b8510,
|
|
36 |
ret=0x7fffe40b84f0) at remote_dispatch.h:7591
|
|
37 |
#9 0x00007ffff73b293f in virNetServerProgramDispatchCall (prog=0x555555ec3ae0,
|
|
38 |
server=0x555555eb7e00, client=0x555555ec4b10, msg=0x555555eb94e0)
|
|
39 |
at rpc/virnetserverprogram.c:435
|
|
40 |
#10 0x00007ffff73b207f in virNetServerProgramDispatch (prog=0x555555ec3ae0,
|
|
41 |
server=0x555555eb7e00, client=0x555555ec4b10, msg=0x555555eb94e0)
|
|
42 |
at rpc/virnetserverprogram.c:305
|
|
43 |
#11 0x00007ffff73a4d2c in virNetServerProcessMsg (srv=0x555555eb7e00, client=0x555555ec4b10,
|
|
44 |
prog=0x555555ec3ae0, msg=0x555555eb94e0) at rpc/virnetserver.c:165
|
|
45 |
#12 0x00007ffff73a4e8d in virNetServerHandleJob (jobOpaque=0x555555ec3e30, opaque=0x555555eb7e00)
|
|
46 |
at rpc/virnetserver.c:186
|
|
47 |
#13 0x00007ffff7187f3f in virThreadPoolWorker (opaque=0x555555eb7ac0) at util/virthreadpool.c:144
|
|
48 |
#14 0x00007ffff718733a in virThreadHelper (data=0x555555eb7890) at util/virthreadpthread.c:161
|
|
49 |
#15 0x00007ffff468ed89 in start_thread (arg=0x7fffec8c0700) at pthread_create.c:308
|
|
50 |
#16 0x00007ffff3da26bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
|
|
51 |
|
|
52 |
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
|
53 |
---
|
|
54 |
src/lxc/lxc_driver.c | 112 +++++++++++++++++++++++++++++++++++++++++++--------
|
|
55 |
1 file changed, 96 insertions(+), 16 deletions(-)
|
|
56 |
|
|
57 |
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
|
|
58 |
index 477d30f..fde05ec 100644
|
|
59 |
--- a/src/lxc/lxc_driver.c
|
|
60 |
+++ b/src/lxc/lxc_driver.c
|
|
61 |
@@ -742,12 +742,24 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
|
|
62 |
int nparams,
|
|
63 |
unsigned int flags)
|
|
64 |
{
|
|
65 |
- size_t i;
|
|
66 |
+ virCapsPtr caps = NULL;
|
|
67 |
+ virDomainDefPtr vmdef = NULL;
|
|
68 |
virDomainObjPtr vm = NULL;
|
|
69 |
+ virLXCDomainObjPrivatePtr priv = NULL;
|
|
70 |
+ virLXCDriverConfigPtr cfg = NULL;
|
|
71 |
+ virLXCDriverPtr driver = dom->conn->privateData;
|
|
72 |
+ unsigned long long hard_limit;
|
|
73 |
+ unsigned long long soft_limit;
|
|
74 |
+ unsigned long long swap_hard_limit;
|
|
75 |
+ bool set_hard_limit = false;
|
|
76 |
+ bool set_soft_limit = false;
|
|
77 |
+ bool set_swap_hard_limit = false;
|
|
78 |
+ int rc;
|
|
79 |
int ret = -1;
|
|
80 |
- virLXCDomainObjPrivatePtr priv;
|
|
81 |
|
|
82 |
- virCheckFlags(0, -1);
|
|
83 |
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
|
|
84 |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
|
|
85 |
+
|
|
86 |
if (virTypedParamsValidate(params, nparams,
|
|
87 |
VIR_DOMAIN_MEMORY_HARD_LIMIT,
|
|
88 |
VIR_TYPED_PARAM_ULLONG,
|
|
89 |
@@ -762,29 +774,97 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
|
|
90 |
goto cleanup;
|
|
91 |
|
|
92 |
priv = vm->privateData;
|
|
93 |
+ cfg = virLXCDriverGetConfig(driver);
|
|
94 |
|
|
95 |
- if (virDomainSetMemoryParametersEnsureACL(dom->conn, vm->def, flags) < 0)
|
|
96 |
+ if (virDomainSetMemoryParametersEnsureACL(dom->conn, vm->def, flags) < 0 ||
|
|
97 |
+ !(caps = virLXCDriverGetCapabilities(driver, false)) ||
|
|
98 |
+ virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
|
|
99 |
+ vm, &flags, &vmdef) < 0)
|
|
100 |
goto cleanup;
|
|
101 |
|
|
102 |
- ret = 0;
|
|
103 |
- for (i = 0; i < nparams; i++) {
|
|
104 |
- virTypedParameterPtr param = ¶ms[i];
|
|
105 |
+ if (flags & VIR_DOMAIN_AFFECT_LIVE &&
|
|
106 |
+ !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
|
|
107 |
+ virReportError(VIR_ERR_OPERATION_INVALID,
|
|
108 |
+ "%s", _("cgroup memory controller is not mounted"));
|
|
109 |
+ goto cleanup;
|
|
110 |
+ }
|
|
111 |
+
|
|
112 |
+#define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \
|
|
113 |
+ if ((rc = virTypedParamsGetULLong(params, nparams, PARAM, &VALUE)) < 0) \
|
|
114 |
+ goto cleanup; \
|
|
115 |
+ \
|
|
116 |
+ if (rc == 1) \
|
|
117 |
+ set_ ## VALUE = true;
|
|
118 |
+
|
|
119 |
+ VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit)
|
|
120 |
+ VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit)
|
|
121 |
+ VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit)
|
|
122 |
+
|
|
123 |
+#undef VIR_GET_LIMIT_PARAMETER
|
|
124 |
+
|
|
125 |
+ /* Swap hard limit must be greater than hard limit.
|
|
126 |
+ * Note that limit of 0 denotes unlimited */
|
|
127 |
+ if (set_swap_hard_limit || set_hard_limit) {
|
|
128 |
+ unsigned long long mem_limit = vm->def->mem.hard_limit;
|
|
129 |
+ unsigned long long swap_limit = vm->def->mem.swap_hard_limit;
|
|
130 |
+
|
|
131 |
+ if (set_swap_hard_limit)
|
|
132 |
+ swap_limit = swap_hard_limit;
|
|
133 |
|
|
134 |
- if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) {
|
|
135 |
- if (virCgroupSetMemoryHardLimit(priv->cgroup, params[i].value.ul) < 0)
|
|
136 |
- ret = -1;
|
|
137 |
- } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) {
|
|
138 |
- if (virCgroupSetMemorySoftLimit(priv->cgroup, params[i].value.ul) < 0)
|
|
139 |
- ret = -1;
|
|
140 |
- } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) {
|
|
141 |
- if (virCgroupSetMemSwapHardLimit(priv->cgroup, params[i].value.ul) < 0)
|
|
142 |
- ret = -1;
|
|
143 |
+ if (set_hard_limit)
|
|
144 |
+ mem_limit = hard_limit;
|
|
145 |
+
|
|
146 |
+ if (virCompareLimitUlong(mem_limit, swap_limit) > 0) {
|
|
147 |
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
|
|
148 |
+ _("memory hard_limit tunable value must be lower "
|
|
149 |
+ "than or equal to swap_hard_limit"));
|
|
150 |
+ goto cleanup;
|
|
151 |
}
|
|
152 |
}
|
|
153 |
|
|
154 |
+#define LXC_SET_MEM_PARAMETER(FUNC, VALUE) \
|
|
155 |
+ if (set_ ## VALUE) { \
|
|
156 |
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) { \
|
|
157 |
+ if ((rc = FUNC(priv->cgroup, VALUE)) < 0) { \
|
|
158 |
+ virReportSystemError(-rc, _("unable to set memory %s tunable"), \
|
|
159 |
+ #VALUE); \
|
|
160 |
+ \
|
|
161 |
+ goto cleanup; \
|
|
162 |
+ } \
|
|
163 |
+ vm->def->mem.VALUE = VALUE; \
|
|
164 |
+ } \
|
|
165 |
+ \
|
|
166 |
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) \
|
|
167 |
+ vmdef->mem.VALUE = VALUE; \
|
|
168 |
+ }
|
|
169 |
+
|
|
170 |
+ /* Soft limit doesn't clash with the others */
|
|
171 |
+ LXC_SET_MEM_PARAMETER(virCgroupSetMemorySoftLimit, soft_limit);
|
|
172 |
+
|
|
173 |
+ /* set hard limit before swap hard limit if decreasing it */
|
|
174 |
+ if (virCompareLimitUlong(vm->def->mem.hard_limit, hard_limit) > 0) {
|
|
175 |
+ LXC_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit);
|
|
176 |
+ /* inhibit changing the limit a second time */
|
|
177 |
+ set_hard_limit = false;
|
|
178 |
+ }
|
|
179 |
+
|
|
180 |
+ LXC_SET_MEM_PARAMETER(virCgroupSetMemSwapHardLimit, swap_hard_limit);
|
|
181 |
+
|
|
182 |
+ /* otherwise increase it after swap hard limit */
|
|
183 |
+ LXC_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit);
|
|
184 |
+
|
|
185 |
+#undef LXC_SET_MEM_PARAMETER
|
|
186 |
+
|
|
187 |
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
|
|
188 |
+ virDomainSaveConfig(cfg->configDir, vmdef) < 0)
|
|
189 |
+ goto cleanup;
|
|
190 |
+
|
|
191 |
+ ret = 0;
|
|
192 |
cleanup:
|
|
193 |
if (vm)
|
|
194 |
virObjectUnlock(vm);
|
|
195 |
+ virObjectUnref(caps);
|
|
196 |
+ virObjectUnref(cfg);
|
|
197 |
return ret;
|
|
198 |
}
|
|
199 |
|