diff mbox series

[ovs-dev,V2,1/1] ofp-parse: Fix segfault due to bad meter n_bands

Message ID 20210317150644.1441938-2-flavio@flaviof.com
State Superseded
Headers show
Series ofp-parse: Fix segfault due to bad meter n_bands | expand

Commit Message

Flaviof March 17, 2021, 3:06 p.m. UTC 
Meter commands internally use ofctl_meter_mod__ and ofctl_meter_request__ functions,
which have an optional parameter called str. When str is NULL, these 2 functions
initialize a struct with meter bands set as NULL. It also needs to set meter
n_bands to 0.

Once del-meters change is added to test dpif-netdev.at is added, the valgrind report on
test '992: dpif-netdev - meters' shows this issue:

   Conditional jump or move depends on uninitialised value(s)
      at 0x473534: ofputil_put_bands (ofp-meter.c:207)
      by 0x473534: ofputil_encode_meter_mod (ofp-meter.c:557)
      by 0x40FBA2: ofctl_meter_mod__ (ovs-ofctl.c:4038)
      by 0x417BD3: ovs_cmdl_run_command__ (command-line.c:247)
      by 0x4078BA: main (ovs-ofctl.c:179)
    Uninitialised value was created by a stack allocation
      at 0x409350: ofctl_del_meters (ovs-ofctl.c:4088)

Fixes: 3200ed5805 ("ovs-ofctl: Add meter support.")
Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
---
v2:
- Use memset to initialize struct instead of setting individial members
- Invoke del-meters in existing tests/dpif-netdev.at test
---
 tests/dpif-netdev.at  | 2 ++
 utilities/ovs-ofctl.c | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 3e6222557..e223446d4 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -370,6 +370,8 @@  recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), a
 recirc_id(0),in_port(8),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), actions:2
 ])
 
+AT_CHECK([ovs-ofctl -O OpenFlow13 del-meters br0])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 3601890f4..62059e962 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -4020,6 +4020,7 @@  ofctl_meter_mod__(const char *bridge, const char *str, int command)
     enum ofputil_protocol usable_protocols;
     enum ofp_version version;
 
+    memset(&mm, 0, sizeof mm);
     if (str) {
         char *error;
         error = parse_ofp_meter_mod_str(&mm, str, command, &usable_protocols);
@@ -4030,7 +4031,6 @@  ofctl_meter_mod__(const char *bridge, const char *str, int command)
         usable_protocols = OFPUTIL_P_OF13_UP;
         mm.command = command;
         mm.meter.meter_id = OFPM13_ALL;
-        mm.meter.bands = NULL;
     }
 
     protocol = open_vconn_for_flow_mod(bridge, &vconn, usable_protocols);
@@ -4050,6 +4050,7 @@  ofctl_meter_request__(const char *bridge, const char *str,
     enum ofputil_protocol protocol;
     enum ofp_version version;
 
+    memset(&mm, 0, sizeof mm);
     if (str) {
         char *error;
         error = parse_ofp_meter_mod_str(&mm, str, -1, &usable_protocols);
@@ -4059,7 +4060,6 @@  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);