If virNWFilterDefToInst returns -1, it has already called virNWFilterInstReset.
Remove the additional call to prevent a double g_free

Found by Linux Verification Center (linuxtesting.org) with Svace.

Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 src/nwfilter/nwfilter_gentech_driver.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 41f270bb7c..f7a909bdc0 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -293,10 +293,8 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverState *driver,
                              tmpvars,
                              useNewFilter,
                              foundNewFilter,
-                             inst) < 0) {
-        virNWFilterInstReset(inst);
+                             inst) < 0)
         return -1;
-    }
 
     return 0;
 }
-- 
2.42.4

On Mon, Apr 14, 2025 at 15:02:35 +0300, Alexander Kuznetsov wrote:
> If virNWFilterDefToInst returns -1, it has already called virNWFilterInstReset.
> Remove the additional call to prevent a double g_free
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  src/nwfilter/nwfilter_gentech_driver.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> index 41f270bb7c..f7a909bdc0 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/src/nwfilter/nwfilter_gentech_driver.c
> @@ -293,10 +293,8 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverState *driver,
>                               tmpvars,
>                               useNewFilter,
>                               foundNewFilter,
> -                             inst) < 0) {
> -        virNWFilterInstReset(inst);
> +                             inst) < 0)
>          return -1;

While this fixes this case originally virNWFilterInstReset could be
called multiple times. This was broken by commit bb4e0596d91, which
replaced VIR_FREE (which clears pointers) to g_free which doesn't in
code paths which do not free the cleared object.

virNWFilterInstReset needs to use g_clear_pointer(..., g_free) instead.

v2:
- switch use g_clear_pointer instead of removing virNWFilterInstReset call

Alexander Kuznetsov (1):
  nwfilter: Avoid possible double free in virNWFilterInstReset()

 src/nwfilter/nwfilter_gentech_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.42.4

virNWFilterInstReset() may be called multiple times, leading to a double g_free()
Replace plain g_free() with g_clear_pointer() to prevent this

Found by Linux Verification Center (linuxtesting.org) with Svace.

Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 src/nwfilter/nwfilter_gentech_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index f7a909bdc0..7462b84f88 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -203,12 +203,12 @@ virNWFilterInstReset(virNWFilterInst *inst)
 
     for (i = 0; i < inst->nfilters; i++)
         virNWFilterObjUnlock(inst->filters[i]);
-    g_free(inst->filters);
+    g_clear_pointer(inst->filters, g_free);
     inst->nfilters = 0;
 
     for (i = 0; i < inst->nrules; i++)
         virNWFilterRuleInstFree(inst->rules[i]);
-    g_free(inst->rules);
+    g_clear_pointer(inst->rules, g_free);
     inst->nrules = 0;
 }
 
-- 
2.42.4

On 4/14/25 15:51, Alexander Kuznetsov wrote:
> virNWFilterInstReset() may be called multiple times, leading to a double g_free()
> Replace plain g_free() with g_clear_pointer() to prevent this
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  src/nwfilter/nwfilter_gentech_driver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
and merged.

Michal