diff mbox series

[ovs-dev,v7,6/8] ovs-vsctl: Fix crash when routing is enabled

Message ID 20230415152155.762025-7-jamestiotio@gmail.com
State Changes Requested, archived
Headers show
Series treewide: Fix multiple Coverity defects | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

James Raphael Tiovalen April 15, 2023, 3:21 p.m. UTC 
In the case where routing is enabled, the bridge member of the
`vsctl_port` structs is not populated. This can cause a crash if we
attempt to access it. This patch fixes the crash by checking if the
bridge member is valid before attempting to access it. In the
`check_conflicts` function, we print both the port name and the bridge
name if routing is disabled and we only print the port name if routing
is enabled.

Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 utilities/ovs-vsctl.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

Simon Horman April 24, 2023, 11:59 a.m. UTC | #1 
On Sat, Apr 15, 2023 at 11:21:53PM +0800, James Raphael Tiovalen wrote:
> In the case where routing is enabled, the bridge member of the
> `vsctl_port` structs is not populated. This can cause a crash if we
> attempt to access it. This patch fixes the crash by checking if the
> bridge member is valid before attempting to access it. In the
> `check_conflicts` function, we print both the port name and the bridge
> name if routing is disabled and we only print the port name if routing
> is enabled.
> 
> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
diff mbox series

Patch

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index b3c75f8ba..f55c2965a 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -888,14 +888,23 @@  check_conflicts(struct vsctl_context *vsctl_ctx, const char *name,
 
     port = shash_find_data(&vsctl_ctx->ports, name);
     if (port) {
-        ctl_fatal("%s because a port named %s already exists on "
-                    "bridge %s", msg, name, port->bridge->name);
+        if (port->bridge) {
+            ctl_fatal("%s because a port named %s already exists on "
+                      "bridge %s", msg, name, port->bridge->name);
+        } else {
+            ctl_fatal("%s because a port named %s already exists", msg, name);
+        }
     }
 
     iface = shash_find_data(&vsctl_ctx->ifaces, name);
     if (iface) {
-        ctl_fatal("%s because an interface named %s already exists "
-                    "on bridge %s", msg, name, iface->port->bridge->name);
+        if (iface->port->bridge) {
+            ctl_fatal("%s because an interface named %s already exists "
+                      "on bridge %s", msg, name, iface->port->bridge->name);
+        } else {
+            ctl_fatal("%s because an interface named %s already exists", msg,
+                      name);
+        }
     }
 
     free(msg);
@@ -935,7 +944,7 @@  find_port(struct vsctl_context *vsctl_ctx, const char *name, bool must_exist)
     ovs_assert(vsctl_ctx->cache_valid);
 
     port = shash_find_data(&vsctl_ctx->ports, name);
-    if (port && !strcmp(name, port->bridge->name)) {
+    if (port && port->bridge && !strcmp(name, port->bridge->name)) {
         port = NULL;
     }
     if (must_exist && !port) {
@@ -953,7 +962,8 @@  find_iface(struct vsctl_context *vsctl_ctx, const char *name, bool must_exist)
     ovs_assert(vsctl_ctx->cache_valid);
 
     iface = shash_find_data(&vsctl_ctx->ifaces, name);
-    if (iface && !strcmp(name, iface->port->bridge->name)) {
+    if (iface && iface->port->bridge &&
+        !strcmp(name, iface->port->bridge->name)) {
         iface = NULL;
     }
     if (must_exist && !iface) {