bridge: Fix qos_unixctl_show bug.

netdev_get_qos returns a value to indicate if an error has occurred while
attempting to query the QoS configuration of an interface. If an error does
occur the pointer argument passed to it will be set to null before returning.
Currently the vswitch will segfault if this occurs as qos_unixctl_show will
attempt to access the pointer directly after it calls netdev_get_qos.

Avoid this by adding a check for the return value and flagging an appropriate
error message to appctl.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
[blp@ovn.org changed details of error report]
Signed-off-by: Ben Pfaff <blp@ovn.org>

diff --git a/AUTHORS b/AUTHORS
index ddab436..c7faeac 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -89,6 +89,7 @@ Helmut Schaa            helmut.schaa@googlemail.com
 Hiteshi Kalra           hiteshi.kalra@tcs.com
 Huanle Han              hanxueluo@gmail.com
 Ian Campbell            Ian.Campbell@citrix.com
+Ian Stokes              ian.stokes@intel.com
 Ilya Maximets           i.maximets@samsung.com
 Isaku Yamahata          yamahata@valinux.co.jp
 James P.                roampune@gmail.com
@@ -311,7 +312,6 @@ Hiroshi Tanaka          htanaka@nicira.com
 Hiroshi Miyata          miyahiro.dazu@gmail.com
 Hsin-Yi Shen            shenh@vmware.com
 Hyojoon Kim             joonk@gatech.edu
-Ian Stokes              ian.stokes@intel.com
 Igor Ganichev           iganichev@nicira.com
 Igor Sever              igor@xorops.com
 Jacob Cherkas           jcherkas@nicira.com

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f518589..8d66c1d 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3130,6 +3130,7 @@ qos_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
     struct iface *iface;
     const char *type;
     struct smap_node *node;
+    int error;
 
     iface = iface_find(argv[1]);
     if (!iface) {
@@ -3137,28 +3138,33 @@ qos_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
         return;
     }
 
-    netdev_get_qos(iface->netdev, &type, &smap);
+    error = netdev_get_qos(iface->netdev, &type, &smap);
+    if (!error) {
+        if (*type != '\0') {
+            struct netdev_queue_dump dump;
+            struct smap details;
+            unsigned int queue_id;
 
-    if (*type != '\0') {
-        struct netdev_queue_dump dump;
-        struct smap details;
-        unsigned int queue_id;
+            ds_put_format(&ds, "QoS: %s %s\n", iface->name, type);
 
-        ds_put_format(&ds, "QoS: %s %s\n", iface->name, type);
+            SMAP_FOR_EACH (node, &smap) {
+                ds_put_format(&ds, "%s: %s\n", node->key, node->value);
+            }
 
-        SMAP_FOR_EACH (node, &smap) {
-            ds_put_format(&ds, "%s: %s\n", node->key, node->value);
-        }
+            smap_init(&details);
+            NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &dump, iface->netdev) {
+                qos_unixctl_show_queue(queue_id, &details, iface, &ds);
+            }
+            smap_destroy(&details);
 
-        smap_init(&details);
-        NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &dump, iface->netdev) {
-            qos_unixctl_show_queue(queue_id, &details, iface, &ds);
+            unixctl_command_reply(conn, ds_cstr(&ds));
+        } else {
+            ds_put_format(&ds, "QoS not configured on %s\n", iface->name);
+            unixctl_command_reply_error(conn, ds_cstr(&ds));
         }
-        smap_destroy(&details);
-
-        unixctl_command_reply(conn, ds_cstr(&ds));
     } else {
-        ds_put_format(&ds, "QoS not configured on %s\n", iface->name);
+        ds_put_format(&ds, "%s: failed to retrieve QOS configuration (%s)\n",
+                      iface->name, ovs_strerror(error));
         unixctl_command_reply_error(conn, ds_cstr(&ds));
     }