| Message ID | 1472738271-14792-1-git-send-email-rmoats@us.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
> On Sep 1, 2016, at 9:57 AM, Ryan Moats <rmoats@us.ibm.com> wrote: > > Found by running valgrind on "ovn-controller - > Chassis external_ids" unit test case: > 24 bytes in 1 blocks are definitely lost in loss record 102 of 180 > at 0x4C2DBB6: malloc (vg_replace_malloc.c:299) > by 0x4916A4: xmalloc (util.c:112) > by 0x47278C: decode_tlv_table_mappings (ofp-util.c:10077) > by 0x472A3A: ofputil_decode_tlv_table_reply (ofp-util.c:10159) > by 0x40C2B1: recv_S_TLV_TABLE_REQUESTED (ofctrl.c:209) > by 0x40C2B1: ofctrl_run (ofctrl.c:471) > by 0x406C8F: main (ovn-controller.c:439) > > Signed-off-by: Ryan Moats <rmoats@us.ibm.com> Acked-by: Flavio Fernandes <flavio@flaviof.com> > --- > ovn/controller/ofctrl.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c > index d8e111d..0493e0d 100644 > --- a/ovn/controller/ofctrl.c > +++ b/ovn/controller/ofctrl.c > @@ -210,6 +210,7 @@ recv_S_TLV_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type) > if (error) { > VLOG_ERR("failed to decode TLV table request (%s)", > ofperr_to_string(error)); > + ofputil_uninit_tlv_table(&reply.mappings); > goto error; > } > > @@ -227,10 +228,12 @@ recv_S_TLV_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type) > "unsupported index %"PRIu16, > map->option_class, map->option_type, > map->option_len, map->index); > + ofputil_uninit_tlv_table(&reply.mappings); > goto error; > } else { > mff_ovn_geneve = MFF_TUN_METADATA0 + map->index; > state = S_CLEAR_FLOWS; > + ofputil_uninit_tlv_table(&reply.mappings); > return; > } > } > @@ -239,6 +242,7 @@ recv_S_TLV_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type) > md_free &= ~(UINT64_C(1) << map->index); > } > } > + ofputil_uninit_tlv_table(&reply.mappings); > > VLOG_DBG("OVN Geneve option not found"); > if (!md_free) { > -- > 2.7.4 (Apple Git-66) > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On Thu, Sep 01, 2016 at 08:57:51AM -0500, Ryan Moats wrote: > Found by running valgrind on "ovn-controller - > Chassis external_ids" unit test case: > 24 bytes in 1 blocks are definitely lost in loss record 102 of 180 > at 0x4C2DBB6: malloc (vg_replace_malloc.c:299) > by 0x4916A4: xmalloc (util.c:112) > by 0x47278C: decode_tlv_table_mappings (ofp-util.c:10077) > by 0x472A3A: ofputil_decode_tlv_table_reply (ofp-util.c:10159) > by 0x40C2B1: recv_S_TLV_TABLE_REQUESTED (ofctrl.c:209) > by 0x40C2B1: ofctrl_run (ofctrl.c:471) > by 0x406C8F: main (ovn-controller.c:439) > > Signed-off-by: Ryan Moats <rmoats@us.ibm.com> The first free here is a double-free because it happens when the decode fails, which leaves the table uninitialized: > @@ -210,6 +210,7 @@ recv_S_TLV_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type) > if (error) { > VLOG_ERR("failed to decode TLV table request (%s)", > ofperr_to_string(error)); > + ofputil_uninit_tlv_table(&reply.mappings); > goto error; > } But honestly the code here is nasty and hard to read. How about: https://patchwork.ozlabs.org/patch/664978/
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index d8e111d..0493e0d 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -210,6 +210,7 @@ recv_S_TLV_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type) if (error) { VLOG_ERR("failed to decode TLV table request (%s)", ofperr_to_string(error)); + ofputil_uninit_tlv_table(&reply.mappings); goto error; } @@ -227,10 +228,12 @@ recv_S_TLV_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type) "unsupported index %"PRIu16, map->option_class, map->option_type, map->option_len, map->index); + ofputil_uninit_tlv_table(&reply.mappings); goto error; } else { mff_ovn_geneve = MFF_TUN_METADATA0 + map->index; state = S_CLEAR_FLOWS; + ofputil_uninit_tlv_table(&reply.mappings); return; } } @@ -239,6 +242,7 @@ recv_S_TLV_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type) md_free &= ~(UINT64_C(1) << map->index); } } + ofputil_uninit_tlv_table(&reply.mappings); VLOG_DBG("OVN Geneve option not found"); if (!md_free) {
Found by running valgrind on "ovn-controller - Chassis external_ids" unit test case: 24 bytes in 1 blocks are definitely lost in loss record 102 of 180 at 0x4C2DBB6: malloc (vg_replace_malloc.c:299) by 0x4916A4: xmalloc (util.c:112) by 0x47278C: decode_tlv_table_mappings (ofp-util.c:10077) by 0x472A3A: ofputil_decode_tlv_table_reply (ofp-util.c:10159) by 0x40C2B1: recv_S_TLV_TABLE_REQUESTED (ofctrl.c:209) by 0x40C2B1: ofctrl_run (ofctrl.c:471) by 0x406C8F: main (ovn-controller.c:439) Signed-off-by: Ryan Moats <rmoats@us.ibm.com> --- ovn/controller/ofctrl.c | 4 ++++ 1 file changed, 4 insertions(+)