| Message ID | CALDO+SYVkkY6e-QjvFkbNKJ4hnmmcGiy58aTdDu5rnCz=44UKw@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Sat, Dec 19, 2015 at 3:07 AM, William Tu <u9012063@gmail.com> wrote: > Hi Ben, > > Thank you for the feedback. > About the wrong free in tun-metadata.c, which one do you mean? It was this one (from the original patch): > --- a/lib/tun-metadata.c > +++ b/lib/tun-metadata.c > @@ -596,6 +596,7 @@ tun_metadata_add_entry(struct tun_table *map, uint8_t > idx, uint16_t opt_class, > > err = tun_metadata_alloc_chain(map, len, cur_chain); > if (err) { > + free(cur_chain); > tun_metadata_del_entry(map, idx); > return OFPERR_NXGTMFC_TABLE_FULL; > } The first time through the loop, cur_chain is initialized to &entry->loc.c and so shouldn't be freed on error. Later instances are malloced and could have resulted in a memory leak. The patch that Ben has applied now fixes the leak.
--- 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); Or this one? --- a/lib/tun-metadata.c +++ b/lib/tun-metadata.c @@ -596,6 +596,7 @@ tun_metadata_add_entry(struct tun_table *map, uint8_t idx, uint16_t opt_class, err = tun_metadata_alloc_chain(map, len, cur_chain); if (err) { + free(cur_chain); tun_metadata_del_entry(map, idx); return OFPERR_NXGTMFC_TABLE_FULL;
Hi Ben, Thank you for the feedback. About the wrong free in tun-metadata.c, which one do you mean? } William On Fri, Dec 18, 2015 at 10:00 PM, Ben Pfaff <blp@ovn.org> wrote: > On Thu, Dec 17, 2015 at 10:39:42AM -0500, Jesse Gross 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> > > Thanks, I applied this to master and branch-2.5. >