External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-07 18:35:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

nwfilter: Fix potential locking problems on ObjLoad failure

Browse Source 
In virNWFilterObjLoad we can still fail after virNWFilterObjAssignDef,
but we don't unlock and free the created virNWFilterObjPtr in the
cleanup path.

The bit we are trying to do after AssignDef is just STRDUP in the
configFile path. However caching the configFile in the NWFilterObj
is largely redundant and doesn't follow the same pattern we use
for domain and network objects.

So just remove all the configFile caching which fixes the latent
bug as a side effect.
This commit is contained in:
Cole Robinson 2016-04-24 18:49:02 -04:00
parent 26af7e4e93
commit ab05abdbc3
3 changed files with 33 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
src/conf/nwfilter_conf.c
View File

@ -353,8 +353,6 @@ virNWFilterObjFree(virNWFilterObjPtr obj)
virNWFilterDefFree(obj->def);
virNWFilterDefFree(obj->newDef);
VIR_FREE(obj->configFile);
virMutexDestroy(&obj->lock);
VIR_FREE(obj);
@ -3181,12 +3179,6 @@ virNWFilterObjLoad(virNWFilterObjListPtr nwfilters,
return NULL;
}
VIR_FREE(nwfilter->configFile); /* for driver reload */
if (VIR_STRDUP(nwfilter->configFile, path) < 0) {
virNWFilterDefFree(def);
return NULL;
}
return nwfilter;
}
@ -3234,60 +3226,66 @@ virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
int
virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
virNWFilterObjPtr nwfilter,
virNWFilterDefPtr def)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *xml;
int ret;
int ret = -1;
char *configFile = NULL;
if (!nwfilter->configFile) {
if (virFileMakePath(driver->configDir) < 0) {
virReportSystemError(errno,
_("cannot create config directory %s"),
driver->configDir);
return -1;
}
if (virFileMakePath(driver->configDir) < 0) {
virReportSystemError(errno,
_("cannot create config directory %s"),
driver->configDir);
goto error;
}
if (!(nwfilter->configFile = virFileBuildPath(driver->configDir,
def->name, ".xml"))) {
return -1;
}
if (!(configFile = virFileBuildPath(driver->configDir,
def->name, ".xml"))) {
goto error;
}
if (!(xml = virNWFilterDefFormat(def))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("failed to generate XML"));
return -1;
goto error;
}
virUUIDFormat(def->uuid, uuidstr);
ret = virXMLSaveFile(nwfilter->configFile,
ret = virXMLSaveFile(configFile,
virXMLPickShellSafeComment(def->name, uuidstr),
"nwfilter-edit", xml);
VIR_FREE(xml);
error:
VIR_FREE(configFile);
return ret;
}
int
virNWFilterObjDeleteDef(virNWFilterObjPtr nwfilter)
virNWFilterObjDeleteDef(const char *configDir,
virNWFilterObjPtr nwfilter)
{
if (!nwfilter->configFile) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("no config file for %s"), nwfilter->def->name);
return -1;
int ret = -1;
char *configFile = NULL;
if (!(configFile = virFileBuildPath(configDir,
nwfilter->def->name, ".xml"))) {
goto error;
}
if (unlink(nwfilter->configFile) < 0) {
if (unlink(configFile) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot remove config for %s"),
nwfilter->def->name);
return -1;
goto error;
}
return 0;
ret = 0;
error:
VIR_FREE(configFile);
return ret;
}

5
src/conf/nwfilter_conf.h
View File

@ -551,7 +551,6 @@ typedef virNWFilterObj *virNWFilterObjPtr;
struct _virNWFilterObj {
virMutex lock;
char *configFile;
int active;
int wantRemoved;
@ -612,10 +611,10 @@ virNWFilterObjPtr virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters,
int virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
virNWFilterObjPtr nwfilter,
virNWFilterDefPtr def);
int virNWFilterObjDeleteDef(virNWFilterObjPtr nwfilter);
int virNWFilterObjDeleteDef(const char *configDir,
virNWFilterObjPtr nwfilter);
virNWFilterObjPtr virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
virNWFilterDefPtr def);

6
src/nwfilter/nwfilter_driver.c
View File

@ -554,7 +554,7 @@ nwfilterDefineXML(virConnectPtr conn,
if (!(nwfilter = virNWFilterObjAssignDef(&driver->nwfilters, def)))
goto cleanup;
if (virNWFilterObjSaveDef(driver, nwfilter, def) < 0) {
if (virNWFilterObjSaveDef(driver, def) < 0) {
virNWFilterObjRemove(&driver->nwfilters, nwfilter);
def = NULL;
goto cleanup;
@ -602,11 +602,9 @@ nwfilterUndefine(virNWFilterPtr obj)
goto cleanup;
}
if (virNWFilterObjDeleteDef(nwfilter) < 0)
if (virNWFilterObjDeleteDef(driver->configDir, nwfilter) < 0)
goto cleanup;
VIR_FREE(nwfilter->configFile);
virNWFilterObjRemove(&driver->nwfilters, nwfilter);
nwfilter = NULL;
ret = 0;