From: Ben Pfaff <blp@nicira.com>
Date: Wed, 20 Jun 2012 17:55:41 +0000 (-0700)
Subject: dpif-linux: Zero 'stats' outputs of dpif_operate() ops on failure.
X-Git-Tag: sliver-openvswitch-1.8.90-0~48^2~322
X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=cfceb2b57a3872a45609e9105f54e0b719bd5ee2;hp=74e60d69adf697e8167a1d1c4d29700ccbe3ee31;p=sliver-openvswitch.git

dpif-linux: Zero 'stats' outputs of dpif_operate() ops on failure.

When DPIF_OP_FLOW_PUT or DPIF_OP_FLOW_DEL operations failed, they left
their 'stats' outputs uninitialized.  For DPIF_OP_FLOW_DEL, this meant that
the caller would read indeterminate data:

Conditional jump or move depends on uninitialised value(s)
   at 0x805C1EB: subfacet_reset_dp_stats (ofproto-dpif.c:4410)
    by 0x80637D2: expire_batch (ofproto-dpif.c:3471)
    by 0x8066114: run (ofproto-dpif.c:3513)
    by 0x8059DF4: ofproto_run (ofproto.c:1035)
    by 0x8052E17: bridge_run (bridge.c:2005)
    by 0x8053F74: main (ovs-vswitchd.c:108)

It's unusual for a delete operation to fail.  The most common reason is an
administrator running "ovs-dpctl del-flows".

The only user of DPIF_OP_FLOW_PUT did not request stats, so this doesn't
fix an actual bug for that case.

Bug #11797.
Reported-by: James Schmidt <jschmidt@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
---

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 62f691744..fcf6899c0 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -968,24 +968,38 @@ dpif_linux_operate__(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
         switch (op->type) {
         case DPIF_OP_FLOW_PUT:
             put = &op->u.flow_put;
-            if (!op->error && put->stats) {
-                struct dpif_linux_flow reply;
-
-                op->error = dpif_linux_flow_from_ofpbuf(&reply, txn->reply);
+            if (put->stats) {
                 if (!op->error) {
-                    dpif_linux_flow_get_stats(&reply, put->stats);
+                    struct dpif_linux_flow reply;
+
+                    op->error = dpif_linux_flow_from_ofpbuf(&reply,
+                                                            txn->reply);
+                    if (!op->error) {
+                        dpif_linux_flow_get_stats(&reply, put->stats);
+                    }
+                }
+
+                if (op->error) {
+                    memset(put->stats, 0, sizeof *put->stats);
                 }
             }
             break;
 
         case DPIF_OP_FLOW_DEL:
             del = &op->u.flow_del;
-            if (!op->error && del->stats) {
-                struct dpif_linux_flow reply;
-
-                op->error = dpif_linux_flow_from_ofpbuf(&reply, txn->reply);
+            if (del->stats) {
                 if (!op->error) {
-                    dpif_linux_flow_get_stats(&reply, del->stats);
+                    struct dpif_linux_flow reply;
+
+                    op->error = dpif_linux_flow_from_ofpbuf(&reply,
+                                                            txn->reply);
+                    if (!op->error) {
+                        dpif_linux_flow_get_stats(&reply, del->stats);
+                    }
+                }
+
+                if (op->error) {
+                    memset(del->stats, 0, sizeof *del->stats);
                 }
             }
             break;