[ovs-dev] [PATCH v2 1/2] ofp-port: Further cleanups and fixes for ofputil_decode_port_stats().

[Ben Pfaff]

This fixes leaks on the error path in parse_intel_port_custom_property().

ofp_print_ofpst_port_reply() failed to free the custom_stats in decoded
port stats.  This fixes the problem.

parse_intel_port_custom_property() had a memory leak if there was more than
one custom stats property (which there shouldn't be, but still).  This
fixes the problem.

There was a function netdev_free_custom_stats_counters() meant for freeing
custom_stats, but hardly anything used it.  This adopts it consistently.

It wasn't safe to free the custom stats if ofputil_decode_port_stats()
returned an error.  Using netdev_free_custom_stats_counters() avoids this
pitfall.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9972
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 lib/ofp-port.c  | 9 ++++++++-
 lib/ofp-print.c | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-port.c b/lib/ofp-port.c
index 8d882a14b4df..ec70f46e96bb 100644
--- a/lib/ofp-port.c
+++ b/lib/ofp-port.c
@@ -1608,6 +1608,7 @@ parse_intel_port_custom_property(struct ofpbuf *payload,
 
     ops->custom_stats.size = ntohs(custom_stats->stats_array_size);
 
+    netdev_free_custom_stats_counters(&ops->custom_stats);
     ops->custom_stats.counters = xcalloc(ops->custom_stats.size,
                                          sizeof *ops->custom_stats.counters);
 
@@ -1618,6 +1619,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) {
+            netdev_free_custom_stats_counters(&ops->custom_stats);
             return OFPERR_OFPBPC_BAD_LEN;
         }
 
@@ -1628,6 +1630,7 @@ parse_intel_port_custom_property(struct ofpbuf *payload,
         /* Counter value. */
         ovs_be64 *value = ofpbuf_try_pull(payload, sizeof *value);
         if (!value) {
+            netdev_free_custom_stats_counters(&ops->custom_stats);
             return OFPERR_OFPBPC_BAD_LEN;
         }
         c->value = ntohll(get_unaligned_be64(value));
@@ -1714,6 +1717,7 @@ ofputil_count_port_stats(const struct ofp_header *oh)
         if (ofputil_decode_port_stats(&ps, &b)) {
             return n;
         }
+        netdev_free_custom_stats_counters(&ps.custom_stats);
     }
 }
 
@@ -1726,7 +1730,10 @@ ofputil_count_port_stats(const struct ofp_header *oh)
  * null and not modify them between calls.
  *
  * Returns 0 if successful, EOF if no replies were left in this 'msg',
- * otherwise a positive errno value. */
+ * otherwise a positive errno value.
+ *
+ * On success, the caller must eventually free ps->custom_stats.counters,
+ * with netdev_free_custom_stats_counters(&ps->custom_stats). */
 int
 ofputil_decode_port_stats(struct ofputil_port_stats *ps, struct ofpbuf *msg)
 {
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index cf93d2e2cb38..e05a969a82b0 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -561,6 +561,7 @@ ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh,
             return retval != EOF ? retval : 0;
         }
         ofputil_format_port_stats(string, &ps, port_map);
+        netdev_free_custom_stats_counters(&ps.custom_stats);
     }
 }
 
-- 
2.16.1