[PATCH] virNWFilterIncludeDefToRuleInst: Prevent potential double g_free
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
participants (3)
-
Alexander Kuznetsov -
Michal Prívozník -
Peter Krempa