On 07/17/2017 04:15 AM, Pavel Hrdina wrote: > On Sat, Jul 15, 2017 at 02:40:06PM -0400, John Ferlan wrote: >> This reverts commit b3e71a8830b2683ee88fa10cb048eabb99a446c0. >> >> As it turns out this ends up very badly as the @def could be Free'd >> even though it's owned by @obj as a result of the AssignDef. > > I don't see a reason to revert it. What do you mean that the @def can > be freed? The virNWFilterObjListAssignDef() doesn't free the @def that > is passed to it, it only assign it to nwfilter object and returns it > immediately. > > Pavel > After ListAssignDef the @def is owned by @obj If the SaveConfig fails, we jump to error: which will free @def. Now we have an @obj in the nwfilters->objs list which has obj->def entry using that address. At some point in the future this will be undefined, which calls virNWFilterObjFree from virNWFilterObjListRemove at which time we'll free something that's already free'd. In between that future free, something else could use obj->def thinking it hasn't been free'd. I realized this as part of the review to the nwfilter patch series John >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> src/conf/virnwfilterobj.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c >> index b5aaa6b..b36eda1 100644 >> --- a/src/conf/virnwfilterobj.c >> +++ b/src/conf/virnwfilterobj.c >> @@ -501,14 +501,14 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters, >> goto error; >> } >> >> - if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) >> - goto error; >> - >> /* We generated a UUID, make it permanent by saving the config to disk */ >> if (!def->uuid_specified && >> virNWFilterSaveConfig(configDir, def) < 0) >> goto error; >> >> + if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) >> + goto error; >> + >> VIR_FREE(configFile); >> return obj; >> >> -- >> 2.9.4 >> >> -- >> libvir-list mailing list >> libvir-list@xxxxxxxxxx >> https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
