ofp-parse: Remove double uninit of group mod if parsing fails.

Double ofputil_uninit_group_mod() used to be harmless, but leads to
double free after commit e8dba7197, which will crash if any error in
group parsing happens.

Add a test to prevent this regression from happening again.

Fixes: e8dba7197 ("meta-flow: Compact struct field_array.")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 0568fc7f688537fda19d2253b553ca3cdb509a57..92c469356fa4dd2a1044671e075a4e976bd43199 100644 (file)
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1733,10 +1733,6 @@ parse_ofp_group_mod_str(struct ofputil_group_mod *gm, int command,
     char *error = parse_ofp_group_mod_str__(gm, command, string,
                                             usable_protocols);
     free(string);
-
-    if (error) {
-        ofputil_uninit_group_mod(gm);
-    }
     return error;
 }
 

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 557c8bee2595d1ed09fe9ccbf89fcf3d3c6c8ace..2978cc5c670b35bea909af3ed5d190eb7f053889 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -418,6 +418,10 @@ AT_CLEANUP
 AT_SETUP([ofproto-dpif - select group with hash selection method])
 OVS_VSWITCHD_START
 add_of_ports br0 1 10 11
+# Check that parse failures after 'fields' parsing work
+AT_CHECK([ovs-ofctl -O OpenFlow10 add-group br0 'group_id=1,type=select,fields(eth_dst),bukket=output:10'], [1], ,[dnl
+ovs-ofctl: unknown keyword bukket
+])
 AT_CHECK([ovs-ofctl -O OpenFlow15 add-group br0 'group_id=1234,type=select,selection_method=hash,fields(eth_dst,ip_dst,tcp_dst),bucket=output:10,bucket=output:11'])
 AT_CHECK([ovs-ofctl -O OpenFlow15 add-flow br0 'ip actions=write_actions(group:1234)'])