[libvirt] [PATCH] Revert "nwfilter: Move save of config until after successful assign"

John Ferlan jferlan at redhat.com
Mon Jul 17 11:39:28 UTC 2017



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




More information about the libvir-list mailing list