dpif-linux: Avoid null dereference if all ports disappear.

[cascardo/ovs.git] / lib / dpif-linux.c

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index abb4b51..076b9ff 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -819,13 +819,19 @@ dpif_linux_port_get_pid__(const struct dpif_linux *dpif, odp_port_t port_no,
     uint32_t port_idx = odp_to_u32(port_no);
     uint32_t pid = 0;
 
-    if (dpif->handlers) {
+    if (dpif->handlers && dpif->uc_array_size > 0) {
         /* The ODPP_NONE "reserved" port number uses the "ovs-system"'s
          * channel, since it is not heavily loaded. */
         uint32_t idx = port_idx >= dpif->uc_array_size ? 0 : port_idx;
         struct dpif_handler *h = &dpif->handlers[hash % dpif->n_handlers];
 
-        pid = nl_sock_pid(h->channels[idx].sock);
+        /* Needs to check in case the socket pointer is changed in between
+         * the holding of upcall_lock.  A known case happens when the main
+         * thread deletes the vport while the handler thread is handling
+         * the upcall from that port. */
+        if (h->channels[idx].sock) {
+            pid = nl_sock_pid(h->channels[idx].sock);
+        }
     }
 
     return pid;
@@ -1041,24 +1047,27 @@ dpif_linux_flow_get__(const struct dpif_linux *dpif,
 static int
 dpif_linux_flow_get(const struct dpif *dpif_,
                     const struct nlattr *key, size_t key_len,
-                    struct ofpbuf **actionsp, struct dpif_flow_stats *stats)
+                    struct ofpbuf **bufp,
+                    struct nlattr **maskp, size_t *mask_len,
+                    struct nlattr **actionsp, size_t *actions_len,
+                    struct dpif_flow_stats *stats)
 {
     const struct dpif_linux *dpif = dpif_linux_cast(dpif_);
     struct dpif_linux_flow reply;
-    struct ofpbuf *buf;
     int error;
 
-    error = dpif_linux_flow_get__(dpif, key, key_len, &reply, &buf);
+    error = dpif_linux_flow_get__(dpif, key, key_len, &reply, bufp);
     if (!error) {
-        if (stats) {
-            dpif_linux_flow_get_stats(&reply, stats);
+        if (maskp) {
+            *maskp = CONST_CAST(struct nlattr *, reply.mask);
+            *mask_len = reply.mask_len;
         }
         if (actionsp) {
-            ofpbuf_set_data(buf, CONST_CAST(struct nlattr *, reply.actions));
-            ofpbuf_set_size(buf, reply.actions_len);
-            *actionsp = buf;
-        } else {
-            ofpbuf_delete(buf);
+            *actionsp = CONST_CAST(struct nlattr *, reply.actions);
+            *actions_len = reply.actions_len;
+        }
+        if (stats) {
+            dpif_linux_flow_get_stats(&reply, stats);
         }
     }
     return error;
@@ -1221,9 +1230,19 @@ dpif_linux_flow_dump_next(const struct dpif *dpif_, void *iter_, void *state_,
         }
 
         if (actions && !state->flow.actions) {
+            struct dpif_linux_flow reply;
+
+            /* Keys are required to be allocated from 'state->buffer' so
+             * they're preserved across calls.  Therefore we need a separate
+             * reply to prevent them from being overwritten.  Actions, however,
+             * don't have this requirement, so it's that fine they're destroyed
+             * on the next call. */
             error = dpif_linux_flow_get__(dpif, state->flow.key,
                                           state->flow.key_len,
-                                          &state->flow, &state->tmp);
+                                          &reply, &state->tmp);
+            state->flow.actions = reply.actions;
+            state->flow.actions_len = reply.actions_len;
+
             if (error == ENOENT) {
                 VLOG_DBG("dumped flow disappeared on get");
             } else if (error) {
@@ -1256,8 +1275,16 @@ static bool
 dpif_linux_flow_dump_next_may_destroy_keys(void *state_)
 {
     struct dpif_linux_flow_state *state = state_;
-
-    return ofpbuf_size(&state->buffer) ? false : true;
+    struct dpif_linux_flow flow;
+    struct ofpbuf nlmsg;
+
+    /* Check whether there's a flow remaining in the buffer that includes
+     * actions.  (If it does not include actions, then we could end up
+     * destroying keys previously returned trying to retrieve its actions
+     * fails.) */
+    return (!nl_dump_peek(&nlmsg, &state->buffer)
+            || dpif_linux_flow_from_ofpbuf(&flow, &nlmsg)
+            || !flow.actions);
 }
 
 static int