Re: [PATCH] qemu: Fix two issues in qemuDomainSetVcpus error handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---

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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: pgpMmO1pqwXkt.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]