dpif: Make it harder to randomly corrupt memory.
authorBen Pfaff <blp@nicira.com>
Thu, 5 Mar 2009 21:57:35 +0000 (13:57 -0800)
committerBen Pfaff <blp@nicira.com>
Thu, 5 Mar 2009 21:57:35 +0000 (13:57 -0800)
lib/dpif.c

index 99337a318de039421dfda675856441a94c14aee2..0f5908fa79d0f298387d8e8d595431be2979c2ee 100644 (file)
@@ -78,6 +78,7 @@ static int open_by_minor(unsigned int minor, struct dpif *);
 static int make_openflow_device(unsigned int minor, char **fnp);
 static char *odp_actions_to_string(const union odp_action actions[],
                                    size_t n_actions);
+static void check_rw_odp_flow(struct odp_flow *);
 
 int
 dpif_open(const char *name, struct dpif *dpif)
@@ -388,12 +389,14 @@ dpif_flow_set_actions(struct dpif *dpif, const struct odp_flow_key *key,
 int
 dpif_flow_del(struct dpif *dpif, struct odp_flow *flow)
 {
+    check_rw_odp_flow(flow);
     return IOCTL(dpif, ODP_FLOW_DEL, flow);
 }
 
 int
 dpif_flow_query(const struct dpif *dpif, struct odp_flow *flow)
 {
+    check_rw_odp_flow(flow);
     return IOCTL(dpif, ODP_FLOW_QUERY, flow);
 }
 
@@ -402,8 +405,13 @@ dpif_flow_query_multiple(const struct dpif *dpif,
                          struct odp_flow flows[], size_t n)
 {
     struct odp_flowvec fv;
+    size_t i;
+
     fv.flows = flows;
     fv.n_flows = n;
+    for (i = 0; i < n; i++) {
+        check_rw_odp_flow(&flows[i]);
+    }
     return IOCTL(dpif, ODP_FLOW_QUERY_MULTIPLE, &fv);
 }
 
@@ -973,3 +981,27 @@ odp_actions_to_string(const union odp_action actions[], size_t n_actions)
     }
     return ds_cstr(&ds);
 }
+
+/* There is a tendency to construct odp_flow objects on the stack and to
+ * forget to properly initialize their "actions" and "n_actions" members.
+ * When this happens, we get memory corruption because the kernel
+ * writes through the random pointer that is in the "actions" member.
+ *
+ * This function attempts to combat the problem by:
+ *
+ *      - Forcing a segfault if "actions" points to an invalid region (instead
+ *        of just getting back EFAULT, which can be easily missed in the log).
+ *
+ *      - Storing a distinctive value that is likely to cause an
+ *        easy-to-identify error later if it is dereferenced, etc.
+ *
+ *      - Triggering a warning on uninitialized memory from Valgrind if
+ *        "actions" or "n_actions" was not initialized.
+ */
+static void
+check_rw_odp_flow(struct odp_flow *flow)
+{
+    if (flow->n_actions) {
+        memset(&flow->actions[0], 0xcc, sizeof flow->actions[0]);
+    }
+}