On 03/07/2014 02:45 AM, Michal Privoznik wrote:
> This fixes a possible double free. In virNetworkAssignDef() if
> virBitmapNew() fails, then virNetworkObjFree(network) is called.
> However, with network->def pointing to actual @def. So if caller
> frees @def again, ...
>
> Moreover, this fixes one possible memory leak too. In
> virInterfaceAssignDef() if appending to the list of interfaces
> fails, we ought to call virInterfaceObjFree() instead of bare
> VIR_FREE().
>
> Although, in order to do that some array size variables needs
> to be turned into size_t rather than int.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 120 +++++++++-----------------------------------
> src/conf/domain_conf.h | 4 +-
> src/conf/interface_conf.c | 26 +++-------
> src/conf/interface_conf.h | 2 +-
> src/conf/network_conf.c | 30 ++++-------
> src/conf/network_conf.h | 4 +-
> src/conf/node_device_conf.c | 16 ++----
> src/conf/node_device_conf.h | 2 +-
> src/conf/nwfilter_conf.c | 33 ++++--------
> src/conf/nwfilter_conf.h | 8 +--
> src/conf/nwfilter_params.c | 12 ++---
> src/conf/nwfilter_params.h | 2 +-
> src/conf/object_event.c | 14 +-----
> src/conf/storage_conf.c | 16 ++----
> src/qemu/qemu_driver.c | 4 +-
> 15 files changed, 76 insertions(+), 217 deletions(-)
>
> @@ -900,14 +887,17 @@ virNetworkDNSHostDefParseXML(const char *networkName,
> if (cur->type == XML_ELEMENT_NODE &&
> xmlStrEqual(cur->name, BAD_CAST "hostname")) {
> if (cur->children != NULL) {
> - if (VIR_REALLOC_N(def->names, def->nnames + 1) < 0)
> - goto error;
> - def->names[def->nnames++] = (char *)xmlNodeGetContent(cur);
> - if (!def->names[def->nnames - 1]) {
> + char *name = (char *) xmlNodeGetContent(cur);
> +
> + if (!name) {
> virReportError(VIR_ERR_XML_DETAIL,
> _("Missing hostname in network '%s' DNS HOST record"),
> networkName);
> }
> + if (VIR_APPEND_ELEMENT(def->names, def->nnames, name) < 0) {
> + VIR_FREE(name);
> + goto error;
> + }
Pre-existing problem - if the xmlNodeGetContent failed, we reported the
error but didn't 'goto error'. Your new code still has the problem; you
need to add a 'goto error' in the 'if (!name)' block.
Amazing how much more compact this is. ACK with the error fix.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list