[ovs-dev] [PATCH v2] pinctrl: Fix memory leak for NAT IPs in send_garp_run().

[Ramu Ramamurthy]

I applied this patch, and while it fixes the original leak, it still
produces the following leak:

 valgrind.8774-==8776== 2,464 bytes in 44 blocks are definitely lost
in loss record 176 of 179
valgrind.8774-==8776==    at 0x4C29BFD: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
valgrind.8774-==8776==    by 0x48E3C7: xmalloc (util.c:112)
valgrind.8774-==8776==    by 0x411BAD: get_nat_addresses_and_keys
(pinctrl.c:1221)
valgrind.8774-==8776==    by 0x411BAD: send_garp_run (pinctrl.c:1259)
valgrind.8774-==8776==    by 0x411BAD: pinctrl_run (pinctrl.c:787)
valgrind.8774:==8776==    by 0x407ABE: main (ovn-controller.c:451)

With the following incremental change, no leak is found in that test.
( as the above points to leaking laddrs which
was mallocd in get_nat_addresses_and_keys())

         struct lport_addresses *laddrs = iter->data;
         destroy_lport_addresses(laddrs);
         shash_delete(&nat_addresses, iter);
+       free(laddrs);
     }
     shash_destroy(&nat_addresses);


On Fri, Aug 19, 2016 at 9:03 AM, Ben Pfaff <blp at ovn.org> wrote:
> send_garp_run() allocated and populated a shash of struct lport_addresses,
> but it only freed some of the data.  This fixes the problem.
>
> CC: Chandra S Vejendla <csvejend at us.ibm.com>
> Reported-by: Ramu Ramamurthy <ramu.ramamurthy at gmail.com>
> Fixes: 8439c2ebd823 ("ovn: Support for GARP for NAT IPs via localnet")
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
> v1->v2: Rebase.
>
>  ovn/controller/pinctrl.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 358602a..fc72bd5 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -1059,7 +1059,6 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
>              }
>              free(name);
>          }
> -        destroy_lport_addresses(laddrs);
>          return;
>      }
>
> @@ -1302,7 +1301,14 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
>      sset_destroy(&localnet_vifs);
>      sset_destroy(&local_l3gw_ports);
>      simap_destroy(&localnet_ofports);
> -    shash_destroy_free_data(&nat_addresses);
> +
> +    SHASH_FOR_EACH_SAFE (iter, next, &nat_addresses) {
> +        struct lport_addresses *laddrs = iter->data;
> +        destroy_lport_addresses(laddrs);
> +        shash_delete(&nat_addresses, iter);
> +    }
> +    shash_destroy(&nat_addresses);
> +
>      sset_destroy(&nat_ip_keys);
>  }
>
> --
> 2.1.3
>