[libvirt] [PATCH] Fix mem leak in virQEMUCapsProbeQMPMachineTypes on OOM
Daniel P. Berrange
berrange at redhat.com
Mon Nov 11 12:39:09 UTC 2013
On Mon, Nov 11, 2013 at 01:28:04PM +0100, Michal Privoznik wrote:
> On 11.11.2013 12:15, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> >
> > The virQEMUCapsProbeQMPMachineTypes method iterates over machine
> > types copying them into the qemuCapsPtr object. It only updates
> > the qemuCaps->nmachinetypes value at the end though. So if OOM
> > occurs in the middle, the destructor of qemuCapsPtr will not
> > free the partially initialized machine types.
> >
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> > src/qemu/qemu_capabilities.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 71a913b..2712a4d 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -2137,14 +2137,17 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
> > goto cleanup;
> >
> > for (i = 0; i < nmachines; i++) {
> > - if (VIR_STRDUP(qemuCaps->machineAliases[i], machines[i]->alias) < 0 ||
> > - VIR_STRDUP(qemuCaps->machineTypes[i], machines[i]->name) < 0)
> > + qemuCaps->nmachineTypes++;
> > + if (VIR_STRDUP(qemuCaps->machineAliases[qemuCaps->nmachineTypes -1],
> > + machines[i]->alias) < 0 ||
> > + VIR_STRDUP(qemuCaps->machineTypes[qemuCaps->nmachineTypes - 1],
> > + machines[i]->name) < 0)
> > goto cleanup;
> > if (machines[i]->isDefault)
> > defIdx = i;
> > - qemuCaps->machineMaxCpus[i] = machines[i]->maxCpus;
> > + qemuCaps->machineMaxCpus[qemuCaps->nmachineTypes - 1] =
> > + machines[i]->maxCpus;
> > }
> > - qemuCaps->nmachineTypes = nmachines;
>
> Wouldn't be sufficient to move this line ^^^ before the for() loop?
> Something like we already have in virQEMUCapsNewCopy().
For this specific case, it would be fine - but the next patch I just
sent causes us to skip some iterations of the loop. So we need to
calculate it inside the loop.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list