[ovs-dev] [PATCH] ofproto-dpif-ipfix: Fix severe memory leak in ipfix_send_template_msgs().

Ben Pfaff blp at ovn.org
Sat May 27 04:14:21 UTC 2017


This fixes a seemingly severe memory leak in ipfix_send_template_msgs().
This function was setting up a buffer with a stub, but only the first 4
or 8 bytes of the stub were actually used because the "sizeof" call used
to size it was actually getting the size of a pointer.  It never freed
the buffer, leaking it.

Additionally, after this code sent a template message, it started over
from the same undersized stub, leaking another block of memory.

This commit fixes both problems.

Found by Coverity.

CC: Romain Lenglet <romain.lenglet at oracle.com>
Reported-at: https://scan3.coverity.com/reports.htm#v16889/p10449/fileInstanceId=14762995&defectInstanceId=4304799&mergedDefectId=180398
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 ofproto/ofproto-dpif-ipfix.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 23fc51b7b053..f8c7ad906acc 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1281,13 +1281,13 @@ ipfix_define_template_fields(enum ipfix_proto_l2 l2, enum ipfix_proto_l3 l3,
 }
 
 static void
-ipfix_init_template_msg(void *msg_stub, uint32_t export_time_sec,
+ipfix_init_template_msg(uint32_t export_time_sec,
                         uint32_t seq_number, uint32_t obs_domain_id,
                         struct dp_packet *msg, size_t *set_hdr_offset)
 {
     struct ipfix_set_header *set_hdr;
 
-    dp_packet_use_stub(msg, msg_stub, sizeof msg_stub);
+    dp_packet_clear(msg);
 
     ipfix_init_header(export_time_sec, seq_number, obs_domain_id, msg);
     *set_hdr_offset = dp_packet_size(msg);
@@ -1322,6 +1322,8 @@ ipfix_send_template_msgs(struct dpif_ipfix_exporter *exporter,
 {
     uint64_t msg_stub[DIV_ROUND_UP(MAX_MESSAGE_LEN, 8)];
     struct dp_packet msg;
+    dp_packet_use_stub(&msg, msg_stub, sizeof msg_stub);
+
     size_t set_hdr_offset, tmpl_hdr_offset, error_pkts;
     struct ipfix_template_record_header *tmpl_hdr;
     uint16_t field_count;
@@ -1332,7 +1334,7 @@ ipfix_send_template_msgs(struct dpif_ipfix_exporter *exporter,
     enum ipfix_proto_l4 l4;
     enum ipfix_proto_tunnel tunnel;
 
-    ipfix_init_template_msg(msg_stub, export_time_sec, exporter->seq_number,
+    ipfix_init_template_msg(export_time_sec, exporter->seq_number,
                             obs_domain_id, &msg, &set_hdr_offset);
     /* Define one template for each possible combination of
      * protocols. */
@@ -1357,7 +1359,7 @@ ipfix_send_template_msgs(struct dpif_ipfix_exporter *exporter,
                         tx_packets += collectors_count(exporter->collectors) - error_pkts;
 
                         /* Reinitialize the template msg. */
-                        ipfix_init_template_msg(msg_stub, export_time_sec,
+                        ipfix_init_template_msg(export_time_sec,
                                                 exporter->seq_number,
                                                 obs_domain_id, &msg,
                                                 &set_hdr_offset);
@@ -1389,6 +1391,7 @@ ipfix_send_template_msgs(struct dpif_ipfix_exporter *exporter,
     /* XXX: Add Options Template Sets, at least to define a Flow Keys
      * Option Template. */
 
+    dp_packet_uninit(&msg);
 }
 
 static inline uint32_t
-- 
2.10.2



More information about the dev mailing list