[ovs-dev] [PATCH] ofproto-dpif-upcall: Fix out-of-scope use of stack memory.

[Alex Wang]

Commit cc377352d (ofproto: Reorganize in preparation for direct
dpdk upcalls.) introduced the bug that keeps reference to 'struct
flow' defined on the stack inside while loop when running out of
the scope.  This causes strange bug like wrong mask extraction
when the part of memory is corrupted, and could lead to even
more serious bug/crash.

This commit fixes the above issue by defining an array of the
'struct flow's on the function stack.

Found by running ovs on RHEL7.

Signed-off-by: Alex Wang <al...@nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 882c067..1f9c484 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -577,6 +577,7 @@ recv_upcalls(struct handler *handler)
     struct ofpbuf recv_bufs[UPCALL_MAX_BATCH];
     struct dpif_upcall dupcalls[UPCALL_MAX_BATCH];
     struct upcall upcalls[UPCALL_MAX_BATCH];
+    struct flow flows[UPCALL_MAX_BATCH];
     size_t n_upcalls, i;
 
     n_upcalls = 0;
@@ -584,8 +585,8 @@ recv_upcalls(struct handler *handler)
         struct ofpbuf *recv_buf = &recv_bufs[n_upcalls];
         struct dpif_upcall *dupcall = &dupcalls[n_upcalls];
         struct upcall *upcall = &upcalls[n_upcalls];
+        struct flow *flow = &flows[n_upcalls];
         struct pkt_metadata md;
-        struct flow flow;
         int error;
 
         ofpbuf_use_stub(recv_buf, recv_stubs[n_upcalls],
@@ -595,13 +596,13 @@ recv_upcalls(struct handler *handler)
             break;
         }
 
-        if (odp_flow_key_to_flow(dupcall->key, dupcall->key_len, &flow)
+        if (odp_flow_key_to_flow(dupcall->key, dupcall->key_len, flow)
             == ODP_FIT_ERROR) {
             goto free_dupcall;
         }
 
         error = upcall_receive(upcall, udpif->backer, &dupcall->packet,
-                               dupcall->type, dupcall->userdata, &flow);
+                               dupcall->type, dupcall->userdata, flow);
         if (error) {
             if (error == ENODEV) {
                 /* Received packet on datapath port for which we couldn't
@@ -611,7 +612,7 @@ recv_upcalls(struct handler *handler)
                 dpif_flow_put(udpif->dpif, DPIF_FP_CREATE, dupcall->key,
                               dupcall->key_len, NULL, 0, NULL, 0, NULL);
                 VLOG_INFO_RL(&rl, "received packet on unassociated datapath "
-                             "port %"PRIu32, flow.in_port.odp_port);
+                             "port %"PRIu32, flow->in_port.odp_port);
             }
             goto free_dupcall;
         }
@@ -621,12 +622,12 @@ recv_upcalls(struct handler *handler)
 
         upcall->out_tun_key = dupcall->out_tun_key;
 
-        if (vsp_adjust_flow(upcall->ofproto, &flow, &dupcall->packet)) {
+        if (vsp_adjust_flow(upcall->ofproto, flow, &dupcall->packet)) {
             upcall->vsp_adjusted = true;
         }
 
-        md = pkt_metadata_from_flow(&flow);
-        flow_extract(&dupcall->packet, &md, &flow);
+        md = pkt_metadata_from_flow(flow);
+        flow_extract(&dupcall->packet, &md, flow);
 
         error = process_upcall(udpif, upcall, NULL);
         if (error) {
-- 
1.7.9.5

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev