Re: [PATCH 02/17] nwfilter: Fix possible corruption on failure path during LoadConfig

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/02/2017 12:25 PM, John Ferlan wrote:
> If the virNWFilterSaveConfig in virNWFilterObjListLoadConfig, then jumping

s/,/ fails,/

> to the error label would free the @def owned by the object, but the object
> is still on the list.
> 
> Fix this by following proper procedure to clear @def and create a new
> variable @objdef to handle the object's def after successful assignment.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/conf/virnwfilterobj.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
> index 0027d45..3fb6046 100644
> --- a/src/conf/virnwfilterobj.c
> +++ b/src/conf/virnwfilterobj.c
> @@ -485,6 +485,7 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters,
>  {
>      virNWFilterDefPtr def = NULL;
>      virNWFilterObjPtr obj;
> +    virNWFilterDefPtr objdef;
>      char *configFile = NULL;
>  
>      if (!(configFile = virFileBuildPath(configDir, name, ".xml")))
> @@ -503,10 +504,12 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters,
>  
>      if (!(obj = virNWFilterObjListAssignDef(nwfilters, def)))
>          goto error;
> +    def = NULL;
> +    objdef = obj->def;
>  
>      /* We generated a UUID, make it permanent by saving the config to disk */
> -    if (!def->uuid_specified &&
> -        virNWFilterSaveConfig(configDir, def) < 0)
> +    if (!objdef->uuid_specified &&
> +        virNWFilterSaveConfig(configDir, objdef) < 0)
>          goto error;

This @objdef variable looks redundant to me. Can't we access obj->def
directly? Esp. since you're introducing a variable just for a two times
use. Then again, we often use obj->def->... when it comes to domain
objects, why not here?

ACK if you drop the @objdef variable.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux