From cb8cdcd78135d73f23a0572bbd6ef3a63de771bc Mon Sep 17 00:00:00 2001 From: Ian Stokes Date: Tue, 8 Mar 2016 23:10:31 +0000 Subject: [PATCH] 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 [blp@ovn.org changed details of error report] Signed-off-by: Ben Pfaff --- AUTHORS | 2 +- vswitchd/bridge.c | 38 ++++++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/AUTHORS b/AUTHORS index d43d9043ed5..d8b6e9d10d8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -76,6 +76,7 @@ Hao Zheng hzheng@nicira.com Helmut Schaa helmut.schaa@googlemail.com Huanle Han hanxueluo@gmail.com Ian Campbell Ian.Campbell@citrix.com +Ian Stokes ian.stokes@intel.com Isaku Yamahata yamahata@valinux.co.jp James P. roampune@gmail.com James Page james.page@ubuntu.com @@ -271,7 +272,6 @@ Henrik Amren henrik@nicira.com Hiroshi Tanaka htanaka@nicira.com Hiroshi Miyata miyahiro.dazu@gmail.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 f8aaa2c7918..f3b13bd5af9 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -3100,6 +3100,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) { @@ -3107,28 +3108,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)); }