[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