dpif-netdev: Introduce port_try_ref() to prevent a race.
authorAlex Wang <alexw@nicira.com>
Thu, 21 Aug 2014 22:54:07 +0000 (15:54 -0700)
committerAlex Wang <alexw@nicira.com>
Wed, 3 Sep 2014 20:29:03 +0000 (13:29 -0700)
When pmd thread interates through all ports for queue loading,
the main thread may unreference and 'rcu-free' a port before
pmd thread take new reference of it.  This could cause pmd
thread fail the reference and access freed memory later.

This commit fixes this race by introducing port_try_ref()
which uses ovs_refcount_try_ref_rcu().  And the pmd thread
will only load the port's queue, if port_try_ref() returns
true.

Found by inspection.

Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
lib/dpif-netdev.c

index 2476435..869fb55 100644 (file)
@@ -880,6 +880,16 @@ port_ref(struct dp_netdev_port *port)
     }
 }
 
+static bool
+port_try_ref(struct dp_netdev_port *port)
+{
+    if (port) {
+        return ovs_refcount_try_ref_rcu(&port->ref_cnt);
+    }
+
+    return false;
+}
+
 static void
 port_destroy__(struct dp_netdev_port *port)
 {
@@ -1864,20 +1874,27 @@ pmd_load_queues(struct pmd_thread *f,
     index = 0;
 
     CMAP_FOR_EACH (port, node, &f->dp->ports) {
-        if (netdev_is_pmd(port->netdev)) {
-            int i;
-
-            for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
-                if ((index % dp->n_pmd_threads) == id) {
-                    poll_list = xrealloc(poll_list, sizeof *poll_list * (poll_cnt + 1));
-
-                    port_ref(port);
-                    poll_list[poll_cnt].port = port;
-                    poll_list[poll_cnt].rx = port->rxq[i];
-                    poll_cnt++;
+        /* Calls port_try_ref() to prevent the main thread
+         * from deleting the port. */
+        if (port_try_ref(port)) {
+            if (netdev_is_pmd(port->netdev)) {
+                int i;
+
+                for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
+                    if ((index % dp->n_pmd_threads) == id) {
+                        poll_list = xrealloc(poll_list,
+                                        sizeof *poll_list * (poll_cnt + 1));
+
+                        port_ref(port);
+                        poll_list[poll_cnt].port = port;
+                        poll_list[poll_cnt].rx = port->rxq[i];
+                        poll_cnt++;
+                    }
+                    index++;
                 }
-                index++;
             }
+            /* Unrefs the port_try_ref(). */
+            port_unref(port);
         }
     }