Message ID | 20180824215014.13635-3-blp@ovn.org |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev,1/4] tests: Add regression tests for all the bugs found by oss-fuzz so far. | expand |
It seems (struct ofputil_port_stats).custom_stats.counters is still leaked by the below code path even after this fix. parse_intel_port_custom_property <- ofputil_pull_ofp14_port_stats <- ofputil_decode_port_stats <- ofputil_count_port_stats I created a diff, how do you like it? Thanks. diff --git a/lib/ofp-port.c b/lib/ofp-port.c index 8d882a14b..b9ad34dbe 100644 --- a/lib/ofp-port.c +++ b/lib/ofp-port.c @@ -1711,7 +1711,9 @@ ofputil_count_port_stats(const struct ofp_header *oh) for (size_t n = 0; ; n++) { struct ofputil_port_stats ps; - if (ofputil_decode_port_stats(&ps, &b)) { + int err = ofputil_decode_port_stats(&ps, &b); + free(ps.custom_stats.counters); + if (err) { return n; } } Yifeng On Fri, Aug 24, 2018 at 2:51 PM Ben Pfaff <blp@ovn.org> wrote: > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9972 > Signed-off-by > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9972Signed-off-by>: > Ben Pfaff <blp@ovn.org> > --- > lib/ofp-port.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/ofp-port.c b/lib/ofp-port.c > index 8d882a14b4df..f19beb64a04c 100644 > --- a/lib/ofp-port.c > +++ b/lib/ofp-port.c > @@ -1618,6 +1618,7 @@ parse_intel_port_custom_property(struct ofpbuf > *payload, > uint8_t *name_len = ofpbuf_try_pull(payload, sizeof *name_len); > char *name = name_len ? ofpbuf_try_pull(payload, *name_len) : > NULL; > if (!name_len || !name) { > + free(ops->custom_stats.counters); > return OFPERR_OFPBPC_BAD_LEN; > } > > @@ -1628,6 +1629,7 @@ parse_intel_port_custom_property(struct ofpbuf > *payload, > /* Counter value. */ > ovs_be64 *value = ofpbuf_try_pull(payload, sizeof *value); > if (!value) { > + free(ops->custom_stats.counters); > return OFPERR_OFPBPC_BAD_LEN; > } > c->value = ntohll(get_unaligned_be64(value)); > -- > 2.16.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Aug 27, 2018 at 04:13:03PM -0700, Yifeng Sun wrote: > It seems (struct ofputil_port_stats).custom_stats.counters is still leaked > by > the below code path even after this fix. > > parse_intel_port_custom_property > <- ofputil_pull_ofp14_port_stats > <- ofputil_decode_port_stats > <- ofputil_count_port_stats > > I created a diff, how do you like it? Thanks. > > diff --git a/lib/ofp-port.c b/lib/ofp-port.c > index 8d882a14b..b9ad34dbe 100644 > --- a/lib/ofp-port.c > +++ b/lib/ofp-port.c > @@ -1711,7 +1711,9 @@ ofputil_count_port_stats(const struct ofp_header *oh) > > for (size_t n = 0; ; n++) { > struct ofputil_port_stats ps; > - if (ofputil_decode_port_stats(&ps, &b)) { > + int err = ofputil_decode_port_stats(&ps, &b); > + free(ps.custom_stats.counters); > + if (err) { > return n; > } > } Thanks, I folded that in.
On Thu, Aug 30, 2018 at 01:15:03PM -0700, Ben Pfaff wrote: > On Mon, Aug 27, 2018 at 04:13:03PM -0700, Yifeng Sun wrote: > > It seems (struct ofputil_port_stats).custom_stats.counters is still leaked > > by > > the below code path even after this fix. > > > > parse_intel_port_custom_property > > <- ofputil_pull_ofp14_port_stats > > <- ofputil_decode_port_stats > > <- ofputil_count_port_stats > > > > I created a diff, how do you like it? Thanks. > > > > diff --git a/lib/ofp-port.c b/lib/ofp-port.c > > index 8d882a14b..b9ad34dbe 100644 > > --- a/lib/ofp-port.c > > +++ b/lib/ofp-port.c > > @@ -1711,7 +1711,9 @@ ofputil_count_port_stats(const struct ofp_header *oh) > > > > for (size_t n = 0; ; n++) { > > struct ofputil_port_stats ps; > > - if (ofputil_decode_port_stats(&ps, &b)) { > > + int err = ofputil_decode_port_stats(&ps, &b); > > + free(ps.custom_stats.counters); > > + if (err) { > > return n; > > } > > } > > Thanks, I folded that in. Actually the freeing should only happen if the decode is successful. There's other stuff wrong in this area too. I'll send a v2.
diff --git a/lib/ofp-port.c b/lib/ofp-port.c index 8d882a14b4df..f19beb64a04c 100644 --- a/lib/ofp-port.c +++ b/lib/ofp-port.c @@ -1618,6 +1618,7 @@ parse_intel_port_custom_property(struct ofpbuf *payload, uint8_t *name_len = ofpbuf_try_pull(payload, sizeof *name_len); char *name = name_len ? ofpbuf_try_pull(payload, *name_len) : NULL; if (!name_len || !name) { + free(ops->custom_stats.counters); return OFPERR_OFPBPC_BAD_LEN; } @@ -1628,6 +1629,7 @@ parse_intel_port_custom_property(struct ofpbuf *payload, /* Counter value. */ ovs_be64 *value = ofpbuf_try_pull(payload, sizeof *value); if (!value) { + free(ops->custom_stats.counters); return OFPERR_OFPBPC_BAD_LEN; } c->value = ntohll(get_unaligned_be64(value));
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9972 Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/ofp-port.c | 2 ++ 1 file changed, 2 insertions(+)