[libvirt] [PATCH v3 python 0/2] fix crash in libvirt_virDomainPin*
this small patch set: * move common logic of all libvirt_virDomainPin* functions to new helper in util module. * fix overrun buffer when converting cpumap. Changes since v2: - fix comments for new helper function - remove extra variables and labels - fix commit message/subject - rename new helper function Changes since v1: - add new helper in util module. Konstantin Neumoin (2): move cpumap conversion code to a common helper don't overrun buffer when converting cpumap libvirt-override.c | 131 +++++------------------------------------------------ libvirt-utils.c | 62 +++++++++++++++++++++++++ libvirt-utils.h | 5 ++ 3 files changed, 78 insertions(+), 120 deletions(-) -- 2.5.5
All libvirt_virDomainPin* functions do the same thing for convert pycpumap to cpumap, so this patch moves all common logic to new helper - virPyCpumapConvert. Signed-off-by: Konstantin Neumoin <kneumoin@virtuozzo.com> --- libvirt-override.c | 131 +++++------------------------------------------------ libvirt-utils.c | 57 +++++++++++++++++++++++ libvirt-utils.h | 5 ++ 3 files changed, 73 insertions(+), 120 deletions(-) diff --git a/libvirt-override.c b/libvirt-override.c index fa3e2ca..be299d4 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -1302,8 +1302,7 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, PyObject *pyobj_domain, *pycpumap; PyObject *ret = NULL; unsigned char *cpumap; - int cpumaplen, vcpu, tuple_size, cpunum; - size_t i; + int cpumaplen, vcpu, cpunum; int i_retval; if (!PyArg_ParseTuple(args, (char *)"OiO:virDomainPinVcpu", @@ -1314,34 +1313,8 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0) return VIR_PY_INT_FAIL; - if (PyTuple_Check(pycpumap)) { - tuple_size = PyTuple_Size(pycpumap); - if (tuple_size == -1) - return ret; - } else { - PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is required"); - return ret; - } - - cpumaplen = VIR_CPU_MAPLEN(cpunum); - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) - return PyErr_NoMemory(); - - for (i = 0; i < tuple_size; i++) { - PyObject *flag = PyTuple_GetItem(pycpumap, i); - bool b; - - if (!flag || libvirt_boolUnwrap(flag, &b) < 0) - goto cleanup; - - if (b) - VIR_USE_CPU(cpumap, i); - else - VIR_UNUSE_CPU(cpumap, i); - } - - for (; i < cpunum; i++) - VIR_UNUSE_CPU(cpumap, i); + if (virPyCpumapConvert(cpunum, pycpumap, &cpumap, &cpumaplen) < 0) + return NULL; LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainPinVcpu(domain, vcpu, cpumap, cpumaplen); @@ -1366,8 +1339,7 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED, PyObject *pyobj_domain, *pycpumap; PyObject *ret = NULL; unsigned char *cpumap; - int cpumaplen, vcpu, tuple_size, cpunum; - size_t i; + int cpumaplen, vcpu, cpunum; unsigned int flags; int i_retval; @@ -1379,34 +1351,8 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED, if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0) return VIR_PY_INT_FAIL; - if (PyTuple_Check(pycpumap)) { - tuple_size = PyTuple_Size(pycpumap); - if (tuple_size == -1) - return ret; - } else { - PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is required"); - return ret; - } - - cpumaplen = VIR_CPU_MAPLEN(cpunum); - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) - return PyErr_NoMemory(); - - for (i = 0; i < tuple_size; i++) { - PyObject *flag = PyTuple_GetItem(pycpumap, i); - bool b; - - if (!flag || libvirt_boolUnwrap(flag, &b) < 0) - goto cleanup; - - if (b) - VIR_USE_CPU(cpumap, i); - else - VIR_UNUSE_CPU(cpumap, i); - } - - for (; i < cpunum; i++) - VIR_UNUSE_CPU(cpumap, i); + if (virPyCpumapConvert(cpunum, pycpumap, &cpumap, &cpumaplen) < 0) + return NULL; LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainPinVcpuFlags(domain, vcpu, cpumap, cpumaplen, flags); @@ -1505,8 +1451,7 @@ libvirt_virDomainPinEmulator(PyObject *self ATTRIBUTE_UNUSED, virDomainPtr domain; PyObject *pyobj_domain, *pycpumap; unsigned char *cpumap = NULL; - int cpumaplen, tuple_size, cpunum; - size_t i; + int cpumaplen, cpunum; int i_retval; unsigned int flags; @@ -1519,37 +1464,9 @@ libvirt_virDomainPinEmulator(PyObject *self ATTRIBUTE_UNUSED, if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0) return VIR_PY_INT_FAIL; - cpumaplen = VIR_CPU_MAPLEN(cpunum); - - if (!PyTuple_Check(pycpumap)) { - PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is required"); - return NULL; - } - - if ((tuple_size = PyTuple_Size(pycpumap)) == -1) + if (virPyCpumapConvert(cpunum, pycpumap, &cpumap, &cpumaplen) < 0) return NULL; - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) - return PyErr_NoMemory(); - - for (i = 0; i < tuple_size; i++) { - PyObject *flag = PyTuple_GetItem(pycpumap, i); - bool b; - - if (!flag || libvirt_boolUnwrap(flag, &b) < 0) { - VIR_FREE(cpumap); - return NULL; - } - - if (b) - VIR_USE_CPU(cpumap, i); - else - VIR_UNUSE_CPU(cpumap, i); - } - - for (; i < cpunum; i++) - VIR_UNUSE_CPU(cpumap, i); - LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainPinEmulator(domain, cpumap, cpumaplen, flags); LIBVIRT_END_ALLOW_THREADS; @@ -1713,8 +1630,7 @@ libvirt_virDomainPinIOThread(PyObject *self ATTRIBUTE_UNUSED, PyObject *pyobj_domain, *pycpumap; PyObject *ret = NULL; unsigned char *cpumap; - int cpumaplen, iothread_val, tuple_size, cpunum; - size_t i; + int cpumaplen, iothread_val, cpunum; unsigned int flags; int i_retval; @@ -1726,33 +1642,8 @@ libvirt_virDomainPinIOThread(PyObject *self ATTRIBUTE_UNUSED, if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0) return VIR_PY_INT_FAIL; - if (PyTuple_Check(pycpumap)) { - if ((tuple_size = PyTuple_Size(pycpumap)) == -1) - return ret; - } else { - PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is required"); - return ret; - } - - cpumaplen = VIR_CPU_MAPLEN(cpunum); - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) - return PyErr_NoMemory(); - - for (i = 0; i < tuple_size; i++) { - PyObject *flag = PyTuple_GetItem(pycpumap, i); - bool b; - - if (!flag || libvirt_boolUnwrap(flag, &b) < 0) - goto cleanup; - - if (b) - VIR_USE_CPU(cpumap, i); - else - VIR_UNUSE_CPU(cpumap, i); - } - - for (; i < cpunum; i++) - VIR_UNUSE_CPU(cpumap, i); + if (virPyCpumapConvert(cpunum, pycpumap, &cpumap, &cpumaplen) < 0) + return NULL; LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainPinIOThread(domain, iothread_val, diff --git a/libvirt-utils.c b/libvirt-utils.c index 2bf7519..09cc1c3 100644 --- a/libvirt-utils.c +++ b/libvirt-utils.c @@ -586,3 +586,60 @@ virPyDictToTypedParams(PyObject *dict, return ret; } #endif /* LIBVIR_CHECK_VERSION(1, 1, 0) */ + + +/* virPyCpumapConvert + * @cpunum: the number of physical cpus of the host. + * @pycpumap: source cpu map, python tuple of bools. + * @cpumapptr: destination cpu map. + * @cpumaplen: destination cpu map length. + * + * Helper function to convert a pycpumap to char*. + * + * Returns 0 on success, -1 on failure with error set. + */ +int +virPyCpumapConvert(int cpunum, + PyObject *pycpumap, + unsigned char **cpumapptr, + int *cpumaplen) +{ + int tuple_size; + size_t i; + *cpumapptr = NULL; + + if (!PyTuple_Check(pycpumap)) { + PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is required"); + return -1; + } + + *cpumaplen = VIR_CPU_MAPLEN(cpunum); + + if ((tuple_size = PyTuple_Size(pycpumap)) == -1) + return -1; + + if (VIR_ALLOC_N(*cpumapptr, *cpumaplen) < 0) { + PyErr_NoMemory(); + return -1; + } + + for (i = 0; i < tuple_size; i++) { + PyObject *flag = PyTuple_GetItem(pycpumap, i); + bool b; + + if (!flag || libvirt_boolUnwrap(flag, &b) < 0) { + VIR_FREE(*cpumapptr); + return -1; + } + + if (b) + VIR_USE_CPU(*cpumapptr, i); + else + VIR_UNUSE_CPU(*cpumapptr, i); + } + + for (; i < cpunum; i++) + VIR_UNUSE_CPU(*cpumapptr, i); + + return 0; +} diff --git a/libvirt-utils.h b/libvirt-utils.h index f74654c..779fd56 100644 --- a/libvirt-utils.h +++ b/libvirt-utils.h @@ -349,4 +349,9 @@ int virPyDictToTypedParams(PyObject *dict, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); # endif /* LIBVIR_CHECK_VERSION(1, 1, 0) */ +int virPyCpumapConvert(int cpunum, + PyObject *pycpumap, + unsigned char **cpumapptr, + int *cpumaplen); + #endif /* __LIBVIRT_UTILS_H__ */ -- 2.5.5
If we pass large(more than cpunum) cpu mask to any libvirt_virDomainPin* function, it could leads to crash. So we have to check tuple size in virPyCpumapConvert and ignore extra tuple members. Signed-off-by: Konstantin Neumoin <kneumoin@virtuozzo.com> --- libvirt-utils.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/libvirt-utils.c b/libvirt-utils.c index 09cc1c3..ac3606b 100644 --- a/libvirt-utils.c +++ b/libvirt-utils.c @@ -623,7 +623,15 @@ virPyCpumapConvert(int cpunum, return -1; } - for (i = 0; i < tuple_size; i++) { + /* Not presented elements of the tuple will be filled by zeros. + * Only first "cpunum" elements make sense, so the rest + * of the bits from the tuple will be ignored. */ + for (i = 0; i < cpunum; i++) { + if (i >= tuple_size) { + VIR_UNUSE_CPU(*cpumapptr, i); + continue; + } + PyObject *flag = PyTuple_GetItem(pycpumap, i); bool b; @@ -638,8 +646,5 @@ virPyCpumapConvert(int cpunum, VIR_UNUSE_CPU(*cpumapptr, i); } - for (; i < cpunum; i++) - VIR_UNUSE_CPU(*cpumapptr, i); - return 0; } -- 2.5.5
On Thu, Nov 03, 2016 at 20:05:52 +0300, Konstantin Neumoin wrote:
If we pass large(more than cpunum) cpu mask to any libvirt_virDomainPin* function, it could leads to crash. So we have to check tuple size in virPyCpumapConvert and ignore extra tuple members.
Signed-off-by: Konstantin Neumoin <kneumoin@virtuozzo.com> --- libvirt-utils.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/libvirt-utils.c b/libvirt-utils.c index 09cc1c3..ac3606b 100644 --- a/libvirt-utils.c +++ b/libvirt-utils.c @@ -623,7 +623,15 @@ virPyCpumapConvert(int cpunum, return -1; }
- for (i = 0; i < tuple_size; i++) { + /* Not presented elements of the tuple will be filled by zeros. + * Only first "cpunum" elements make sense, so the rest + * of the bits from the tuple will be ignored. */ + for (i = 0; i < cpunum; i++) { + if (i >= tuple_size) { + VIR_UNUSE_CPU(*cpumapptr, i);
You don't really need to UNUSE the cpus since the array was cleared when allocated. I'll tweak it and push the patch in a while. Thanks for fixing the bug. Peter
participants (2)
-
Konstantin Neumoin -
Peter Krempa