@@ -754,6 +754,8 @@ parse_ofp_packet_out_str(struct ofputil_packet_out *po, const char *str_,
return error;
}
+/* Parse a string representation of a meter modification message to '*mm'.
+ * If successful, 'mm->meter.bands' must be free()d by the caller. */
static char * OVS_WARN_UNUSED_RESULT
parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string,
struct ofpbuf *bands, int command,
@@ -795,6 +797,9 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string,
mm->command = command;
mm->meter.meter_id = 0;
mm->meter.flags = 0;
+ mm->meter.n_bands = 0;
+ mm->meter.bands = NULL;
+
if (fields & F_BANDS) {
band_str = strstr(string, "band");
if (!band_str) {
@@ -945,9 +950,6 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string,
}
}
}
- } else {
- mm->meter.n_bands = 0;
- mm->meter.bands = NULL;
}
return NULL;
@@ -957,7 +959,8 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string,
* page) into 'mm' for sending the specified meter_mod 'command' to a switch.
*
* Returns NULL if successful, otherwise a malloc()'d string describing the
- * error. The caller is responsible for freeing the returned string. */
+ * error. The caller is responsible for freeing the returned string.
+ * If successful, 'mm->meter.bands' must be free()d by the caller. */
char * OVS_WARN_UNUSED_RESULT
parse_ofp_meter_mod_str(struct ofputil_meter_mod *mm, const char *str_,
int command, enum ofputil_protocol *usable_protocols)
@@ -3704,6 +3704,7 @@ ofctl_meter_mod__(const char *bridge, const char *str, int command)
protocol = open_vconn_for_flow_mod(bridge, &vconn, usable_protocols);
version = ofputil_protocol_to_ofp_version(protocol);
transact_noreply(vconn, ofputil_encode_meter_mod(version, &mm));
+ free(mm.meter.bands);
vconn_close(vconn);
}
@@ -3726,12 +3727,14 @@ ofctl_meter_request__(const char *bridge, const char *str,
} else {
usable_protocols = OFPUTIL_P_OF13_UP;
mm.meter.meter_id = OFPM13_ALL;
+ mm.meter.bands = NULL;
}
protocol = open_vconn_for_flow_mod(bridge, &vconn, usable_protocols);
version = ofputil_protocol_to_ofp_version(protocol);
dump_transaction(vconn, ofputil_encode_meter_request(version, type,
mm.meter.meter_id));
+ free(mm.meter.bands);
vconn_close(vconn);
}
The function parse_ofp_meter_mod_str() allocates a buffer called 'bands', which parse_ofp_meter_mod_str__() then steals for the member 'mm->meter.bands'. Calling functions didn't free that stolen value and the comments for those function didn't indicate that was necessary. Found by valgrind. Signed-off-by: Justin Pettit <jpettit@ovn.org> --- v1->v2: Fix unitialized path. --- lib/ofp-parse.c | 11 +++++++---- utilities/ovs-ofctl.c | 3 +++ 2 files changed, 10 insertions(+), 4 deletions(-)