Re: [libvirt] [PATCH] Revert "nwfilter: Move save of config until after successful assign"
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(a)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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list