[ovs-dev] [PATCH] route-table: Fix memory leak reported by valgrind.

Thadeu Lima de Souza Cascardo cascardo at redhat.com
Mon Jun 20 19:08:41 UTC 2016


On Mon, Jun 20, 2016 at 07:32:52AM -0700, William Tu wrote:
> Testcase 2050, ovn -- 3 HVs, 1 LS, 3 lports/HV, reports possible leak:
>     nln_notifier_create (netlink-notifier.c:131)
>     name_table_init (route-table.c:319)
>     route_table_init (route-table.c:110)
>     dp_initialize (dpif.c:126)
>     dp_unregister_provider (dpif.c:218)
>     dpif_dummy_override (dpif-netdev.c:4309)
>     dpif_dummy_register (dpif-netdev.c:4329)
>     dummy_enable (dummy.c:46)
>     parse_options (ovs-vswitchd.c:205)
>     main (ovs-vswitchd.c:76)
> 'name_notifier' could be overwritten without being free'd.
> 
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/138910851
> Signed-off-by: William Tu <u9012063 at gmail.com>
> ---
>  lib/route-table.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/route-table.c b/lib/route-table.c
> index 58e7f62..cf01c34 100644
> --- a/lib/route-table.c
> +++ b/lib/route-table.c
> @@ -316,6 +316,7 @@ route_table_fallback_lookup(const struct in6_addr *ip6_dst OVS_UNUSED,
>  static void
>  name_table_init(void)
>  {
> +    free(name_notifier);
>      name_notifier = rtnetlink_notifier_create(name_table_change, NULL);
>  }

That doesn't seem right. I could not reproduce the valgrind problem by running
TESTSUITEFLAGS=2050 make check-valgrind.

But this has several problems. Fist, it's not clear code. Second, if
name_notifier was not NULL, it could release memory still in use, and cause
other potential bugs and leaks as well.

Third: it's not even necessary. This looks much more like a false positive from
valgrind. Unless we are calling name_table_init twice, which I can't see how.
Can you look into the real bug here, maybe WARN whenever name_table_init when
name_notifier is not NULL?

If this is really a false positive and you really want to get rid of it, you
could just do:

  static void
  name_table_init(void)
  {
 -    name_notifier = rtnetlink_notifier_create(name_table_change, NULL);
 +    if (name_notifier == NULL) {
 +        name_notifier = rtnetlink_notifier_create(name_table_change, NULL);
 +    }
  }

Cascardo.

>  
> -- 
> 2.5.0
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list