[libvirt] [PATCH v2] nwfilter: Don't leak @inetaddr

ZhiPeng Lu posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1506455646-2452-1-git-send-email-lu.zhipeng@zte.com.cn
There is a newer version of this series
src/nwfilter/nwfilter_learnipaddr.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[libvirt] [PATCH v2] nwfilter: Don't leak @inetaddr
Posted by ZhiPeng Lu 6 years, 6 months ago
In learnIPAddressThread()the @inetaddr may be leaked.

Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>
---
 src/nwfilter/nwfilter_learnipaddr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index cfd92d9..5dc212e 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -625,6 +625,7 @@ learnIPAddressThread(void *arg)
             if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) {
                 VIR_ERROR(_("Failed to add IP address %s to IP address "
                           "cache for interface %s"), inetaddr, req->ifname);
+                VIR_FREE(inetaddr);
             }
 
             ret = virNWFilterInstantiateFilterLate(req->driver,
@@ -636,7 +637,8 @@ learnIPAddressThread(void *arg)
                                                    req->filtername,
                                                    req->filterparams);
             VIR_DEBUG("Result from applying firewall rules on "
-                      "%s with IP addr %s : %d", req->ifname, inetaddr, ret);
+                      "%s with IP addr %s : %d", req->ifname, NULLSTR(inetaddr), ret);
+
         }
     } else {
         if (showError)
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: Don't leak @inetaddr
Posted by John Ferlan 6 years, 6 months ago

On 09/26/2017 03:54 PM, ZhiPeng Lu wrote:
> In learnIPAddressThread()the @inetaddr may be leaked.
> 
> Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>
> ---
>  src/nwfilter/nwfilter_learnipaddr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
> index cfd92d9..5dc212e 100644
> --- a/src/nwfilter/nwfilter_learnipaddr.c
> +++ b/src/nwfilter/nwfilter_learnipaddr.c
> @@ -625,6 +625,7 @@ learnIPAddressThread(void *arg)
>              if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) {
>                  VIR_ERROR(_("Failed to add IP address %s to IP address "
>                            "cache for interface %s"), inetaddr, req->ifname);
> +                VIR_FREE(inetaddr);
>              }
>  
>              ret = virNWFilterInstantiateFilterLate(req->driver,
> @@ -636,7 +637,8 @@ learnIPAddressThread(void *arg)
>                                                     req->filtername,
>                                                     req->filterparams);
>              VIR_DEBUG("Result from applying firewall rules on "
> -                      "%s with IP addr %s : %d", req->ifname, inetaddr, ret);
> +                      "%s with IP addr %s : %d", req->ifname, NULLSTR(inetaddr), ret);> +

Still not quite right... VIR_FREE() only happens if
virNWFilterIPAddrMapAddIPAddr() < 0.

Not sure what the purpose of a VIR_FREE in the upper condition and then
NULLSTR() below would be...  We're still calling
virNWFilterInstantiateFilterLate regardless and still want the VIR_DEBUG
printed.

Perhaps just a VIR_FREE() after the VIR_DEBUG would seem to be
sufficient since there's no escape clause.  The VIR_ERROR may help us
understand why/if ret != 0 though... I didn't put much thought into that
though.

John

>          }
>      } else {
>          if (showError)
> 

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