[libvirt] [PATCH] qemu: Fix two issues in qemuDomainSetVcpus error handling
Martin Kletzander
mkletzan at redhat.com
Wed Mar 18 12:20:20 UTC 2015
On Wed, Mar 18, 2015 at 07:25:29AM -0400, John Ferlan wrote:
>Issue #1 - A call to virBitmapNew did not check if the allocation
>failed which could lead to a NULL dereference
>
>Issue #2 - When deleting the pin entries from the config file, the
>code loops from the number of elements down to the "new" vcpu count;
>however, the pin id values are numbered 0..n-1 not 1..n, so the "first"
>pin attempt would never work. Luckily the check was for whether the
>incoming 'n' (vcpu id) matched the entry in the array from 0..arraysize
>rather than a dereference of the 'n' entry
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
>
>NOTE: These were found on inspection while working/debugging the a IOThreads
>series which borrows from the SetVcpus code. I can separate the two if
>desired, but I think the second issue is mostly an optimization.
>
If you want to, but in this particular case I think it has no difference.
>
> src/qemu/qemu_driver.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index ed6764d..6d9217b 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -4752,7 +4752,11 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
> if (VIR_ALLOC(vcpupin) < 0)
> goto cleanup;
>
>- vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN);
>+ if (!(vcpupin->cpumask =
>+ virBitmapNew(VIR_DOMAIN_CPUMASK_LEN))) {
>+ VIR_FREE(vcpupin);
>+ goto cleanup;
>+ }
> virBitmapCopy(vcpupin->cpumask, vm->def->cpumask);
> vcpupin->id = i;
> if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin,
>@@ -4987,7 +4991,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> /* remove vcpupin entries for vcpus that were unplugged */
> if (nvcpus < persistentDef->vcpus) {
>- for (i = persistentDef->vcpus; i >= nvcpus; i--)
>+ for (i = persistentDef->vcpus - 1; i >= nvcpus; i--)
for (i = nvcpus; i < persistentDef->vcpus, i++)
would be easily readable (I guess the construct in the code might be
just a leftover when it was an array).
ACK with any of the versions.
> virDomainPinDel(&persistentDef->cputune.vcpupin,
> &persistentDef->cputune.nvcpupin,
> i);
>--
>2.1.0
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150318/385a24fa/attachment-0001.sig>
More information about the libvir-list
mailing list