[ovs-dev] [leaks 1/4] ofproto-dpif: Avoid potential undefined behavior in type_run().

Ben Pfaff blp at nicira.com
Tue Dec 18 21:51:02 UTC 2012


When HMAP_FOR_EACH completes, the value in the loop control variable is not
necessarily NULL.  It is NULL minus the offset of the hmap_node struct
member, which is nonnull if that offset is nonzero.  Currently,
'all_ofproto_dpifs_node' is the first member in struct ofproto_dpif, so
there is no real bug, but there would be if the struct were rearranged.

This commit heads off the problem by avoiding any assumption about the
loop control variable after HMAP_FOR_EACH.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ofproto/ofproto-dpif.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ca0a065..95fd54e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -794,6 +794,20 @@ port_open_type(const char *datapath_type, const char *port_type)
 
 /* Type functions. */
 
+static struct ofproto_dpif *
+lookup_ofproto_dpif_by_port_name(const char *name)
+{
+    struct ofproto_dpif *ofproto;
+
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        if (sset_contains(&ofproto->ports, name)) {
+            return ofproto;
+        }
+    }
+
+    return NULL;
+}
+
 static int
 type_run(const char *type)
 {
@@ -817,7 +831,7 @@ type_run(const char *type)
 
     /* Check for port changes in the dpif. */
     while ((error = dpif_port_poll(backer->dpif, &devname)) == 0) {
-        struct ofproto_dpif *ofproto = NULL;
+        struct ofproto_dpif *ofproto;
         struct dpif_port port;
 
         /* Don't report on the datapath's device. */
@@ -825,13 +839,7 @@ type_run(const char *type)
             continue;
         }
 
-        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node,
-                       &all_ofproto_dpifs) {
-            if (sset_contains(&ofproto->ports, devname)) {
-                break;
-            }
-        }
-
+        ofproto = lookup_ofproto_dpif_by_port_name(devname);
         if (dpif_port_query_by_name(backer->dpif, devname, &port)) {
             /* The port was removed.  If we know the datapath,
              * report it through poll_set().  If we don't, it may be
-- 
1.7.2.5




More information about the dev mailing list