[ovs-dev] [PATCH ovn] pinctrl: Fix race condition when accessing br_int_name.

[Dumitru Ceara]

Reading pctrl->br_int_name must be done while holding the pinctrl_mutex.
Otherwise we risk accessing freed memory.

Fixes: 6b72068202f1 ("ovn-controller: Add a new thread in pinctrl module to handle packet-ins.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
 controller/pinctrl.c | 45 ++++++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 919eae4..e56ccbc 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -3011,6 +3011,23 @@ notify_pinctrl_main(void)
     seq_change(pinctrl_main_seq);
 }
 
+static void
+pinctrl_rconn_setup(struct rconn *swconn, const char *br_int_name)
+    OVS_REQUIRES(pinctrl_mutex)
+{
+    if (br_int_name) {
+        char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int_name);
+
+        if (strcmp(target, rconn_get_target(swconn))) {
+            VLOG_INFO("%s: connecting to switch", target);
+            rconn_connect(swconn, target, target);
+        }
+        free(target);
+    } else {
+        rconn_disconnect(swconn);
+    }
+}
+
 /* pinctrl_handler pthread function. */
 static void *
 pinctrl_handler(void *arg_)
@@ -3022,7 +3039,6 @@ pinctrl_handler(void *arg_)
      * rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
     unsigned int conn_seq_no = 0;
 
-    char *br_int_name = NULL;
     uint64_t new_seq;
 
     /* Next IPV6 RA in seconds. */
@@ -3037,27 +3053,8 @@ pinctrl_handler(void *arg_)
     swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
 
     while (!latch_is_set(&pctrl->pinctrl_thread_exit)) {
-        if (pctrl->br_int_name) {
-            if (!br_int_name || strcmp(br_int_name, pctrl->br_int_name)) {
-                free(br_int_name);
-                br_int_name = xstrdup(pctrl->br_int_name);
-            }
-        }
-
-        if (br_int_name) {
-            char *target;
-
-            target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int_name);
-            if (strcmp(target, rconn_get_target(swconn))) {
-                VLOG_INFO("%s: connecting to switch", target);
-                rconn_connect(swconn, target, target);
-            }
-            free(target);
-        } else {
-            rconn_disconnect(swconn);
-        }
-
         ovs_mutex_lock(&pinctrl_mutex);
+        pinctrl_rconn_setup(swconn, pctrl->br_int_name);
         ip_mcast_snoop_run();
         ovs_mutex_unlock(&pinctrl_mutex);
 
@@ -3113,19 +3110,17 @@ pinctrl_handler(void *arg_)
         poll_block();
     }
 
-    free(br_int_name);
     rconn_destroy(swconn);
     return NULL;
 }
 
 static void
 pinctrl_set_br_int_name_(char *br_int_name)
+    OVS_REQUIRES(pinctrl_mutex)
 {
     if (br_int_name && (!pinctrl.br_int_name || strcmp(pinctrl.br_int_name,
                                                        br_int_name))) {
-        if (pinctrl.br_int_name) {
-            free(pinctrl.br_int_name);
-        }
+        free(pinctrl.br_int_name);
         pinctrl.br_int_name = xstrdup(br_int_name);
         /* Notify pinctrl_handler that integration bridge is
          * set/changed. */
-- 
1.8.3.1