stp: Initialize mutex whenever we register unixctl command.

The stp/tcn command, which locks the mutex, was being registered without
initializing the mutex, so calling stp/tcn before STP was enabled on the
switch caused a crash.  This commit fixes the bug by initializing the mutex
at the same time we register the stp/tcn command.

Reported-by: Ding Zhi <zhi.ding@6wind.com>
Reported-at: http://openvswitch.org/pipermail/dev/2016-May/071381.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
Tested-by: Quentin Monnet <quentin.monnet@6wind.com>

2 files changed, 16 insertions, 12 deletions

diff --git a/AUTHORS b/AUTHORS
index 5a075d5e2..17179bc7e 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -185,6 +185,7 @@ David Palma             palma@onesource.pt
 Derek Cormier           derek.cormier@lab.ntt.co.jp
 Dhaval Badiani          dbadiani@vmware.com
 DK Moon                 dkmoon@nicira.com
+Ding Zhi                zhi.ding@6wind.com
 Edwin Chiu              echiu@vmware.com
 Eivind Bulie Haanaes
 Eric Lopez              elopez@nicira.com
diff --git a/lib/stp.c b/lib/stp.c
index 1d395f302..2b7e6260c 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -235,8 +235,19 @@ static void stp_unixctl_tcn(struct unixctl_conn *, int argc,
 void
 stp_init(void)
 {
-    unixctl_command_register("stp/tcn", "[bridge]", 0, 1, stp_unixctl_tcn,
-                             NULL);
+    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+    if (ovsthread_once_start(&once)) {
+        /* We need a recursive mutex because stp_send_bpdu() could loop back
+         * into the stp module through a patch port.  This happens
+         * intentionally as part of the unit tests.  Ideally we'd ditch
+         * the call back function, but for now this is what we have. */
+        ovs_mutex_init_recursive(&mutex);
+
+        unixctl_command_register("stp/tcn", "[bridge]", 0, 1, stp_unixctl_tcn,
+                                 NULL);
+        ovsthread_once_done(&once);
+    }
 }
 
 /* Creates and returns a new STP instance that initially has no ports enabled.
@@ -257,18 +268,10 @@ stp_create(const char *name, stp_identifier bridge_id,
            void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void *aux),
            void *aux)
 {
-    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
     struct stp *stp;
     struct stp_port *p;
 
-    if (ovsthread_once_start(&once)) {
-        /* We need a recursive mutex because stp_send_bpdu() could loop back
-         * into the stp module through a patch port.  This happens
-         * intentionally as part of the unit tests.  Ideally we'd ditch
-         * the call back function, but for now this is what we have. */
-        ovs_mutex_init_recursive(&mutex);
-        ovsthread_once_done(&once);
-    }
+    stp_init();
 
     ovs_mutex_lock(&mutex);
     stp = xzalloc(sizeof *stp);