From 7419085a13ea530c3450bcb7f60ae1a6ec5365c3 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Thu, 5 Mar 2009 13:57:35 -0800
Subject: [PATCH] dpif: Make it harder to randomly corrupt memory.

---
 lib/dpif.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/lib/dpif.c b/lib/dpif.c
index 99337a31..0f5908fa 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -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]);
+    }
+}
-- 
2.30.2