diff mbox

[ovs-dev] tun-metadata: Fix memory leak in tun_metadata_add_entry() corner case.

Message ID CALDO+SYbckB8khGOGj4foGrBYaxBf2KBLkT62X_-jLuhUYEitQ@mail.gmail.com
State Not Applicable
Headers show

Commit Message

William Tu Dec. 17, 2015, 10:28 p.m. UTC
Hi Ben,

I've applied your patch and valgrind no longer complain about
tun_metadata_add_entry. However, another issue found (reported by testcase
643), patch below provided the fix and testcase 643 passes.

valgrind message
==20490==    by 0x4E1384: xmalloc (util.c:112)
==20490==    by 0x468CD1: resize (hmap.c:100)
==20490==    by 0x4DE87B: hmap_insert_at (hmap.h:235)
==20490==    by 0x4DE87B: table_alloc (tun-metadata.c:106)
==20490==    by 0x4DEF93: tun_metadata_table_mod (tun-metadata.c:164)
==20490==    by 0x420D85: handle_geneve_table_mod (ofproto.c:7080)
==20490==    by 0x420D85: handle_openflow__ (ofproto.c:7245)

patch:






On Thu, Dec 17, 2015 at 7:39 AM, Jesse Gross <jesse@kernel.org> wrote:

> On Thu, Dec 17, 2015 at 4:55 AM, Ben Pfaff <blp@ovn.org> wrote:
> > Found by valgrind.
> >
> > CC: Jesse Gross <jgross@vmware.com>
> > Reported-by: William Tu <u9012063@gmail.com>
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
>
> Acked-by: Jesse Gross <jesse@kernel.org>
>

Comments

Jesse Gross Dec. 17, 2015, 10:52 p.m. UTC | #1
On Thu, Dec 17, 2015 at 5:28 PM, Cheng-Chun Tu <u9012063@gmail.com> wrote:
> Hi Ben,
>
> I've applied your patch and valgrind no longer complain about
> tun_metadata_add_entry. However, another issue found (reported by testcase
> 643), patch below provided the fix and testcase 643 passes.
>
> valgrind message
> ==20490==    by 0x4E1384: xmalloc (util.c:112)
> ==20490==    by 0x468CD1: resize (hmap.c:100)
> ==20490==    by 0x4DE87B: hmap_insert_at (hmap.h:235)
> ==20490==    by 0x4DE87B: table_alloc (tun-metadata.c:106)
> ==20490==    by 0x4DEF93: tun_metadata_table_mod (tun-metadata.c:164)
> ==20490==    by 0x420D85: handle_geneve_table_mod (ofproto.c:7080)
> ==20490==    by 0x420D85: handle_openflow__ (ofproto.c:7245)

This patch looks right to me, can you submit it formally with a
signed-off-by? Thanks for tracking all of these down!
diff mbox

Patch

diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
index aa5b48d..7f467a9 100644
--- a/lib/tun-metadata.c
+++ b/lib/tun-metadata.c
@@ -131,7 +131,7 @@  table_free(struct tun_table *map)
OVS_REQUIRES(tab_mutex)
     HMAP_FOR_EACH (entry, node, &map->key_hmap) {
         tun_metadata_del_entry(map, entry - map->entries);
     }
-
+       hmap_destroy(&map->key_hmap);
     free(map);
 }