Re: [ovs-dev] [PATCH 3/4] ofp-port: Fix leak on error path in parse_intel_port_custom_property().

2018-08-30 Thread Ben Pfaff
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(, )) {
> > +   int err = ofputil_decode_port_stats(, );
> > +   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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 3/4] ofp-port: Fix leak on error path in parse_intel_port_custom_property().

2018-08-30 Thread Ben Pfaff
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(, )) {
> +   int err = ofputil_decode_port_stats(, );
> +   free(ps.custom_stats.counters);
> +   if (err) {
>  return n;
>  }
>  }

Thanks, I folded that in.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 3/4] ofp-port: Fix leak on error path in parse_intel_port_custom_property().

2018-08-27 Thread Yifeng Sun
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(, )) {
+   int err = ofputil_decode_port_stats(, );
+   free(ps.custom_stats.counters);
+   if (err) {
 return n;
 }
 }


Yifeng


On Fri, Aug 24, 2018 at 2:51 PM Ben Pfaff  wrote:

> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9972
> Signed-off-by
> :
> Ben Pfaff 
> ---
>  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
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH 3/4] ofp-port: Fix leak on error path in parse_intel_port_custom_property().

2018-08-24 Thread Ben Pfaff
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9972
Signed-off-by: Ben Pfaff 
---
 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev