Free the defer data in case of errors

To make sure that we flush all the packets that are queued in a flow
entry, we run a defer function. If for any reason this defer was not
scheduled (because the function was called with no hold queue), the
defer data has to be freed.

Closes-BUG: #1436798
(cherry picked from commit 8c30ce9)

Fix improper boundary checks and reference count leaks

Boundary checks allow for one extra label than the maximum, causing
memory corruption. Also, when a label is changed, reference to old
nexthop has to be released. Two harmless boundary checks in nexthop
subysystem is also addressed.

Closes-BUG: #1446550
Change-Id: I9289265b8a843160fdfe6fffc3e52c131d9b2a4a

dp-core/vr_flow.c

@@ -106,7 +106,7 @@ vr_valid_link_local_port(struct vrouter *router, int family,
     if (proto == VR_IP_PROTO_UDP)
         tmp += (router->vr_link_local_ports_size * 8 / 2);
 
-    data = router->vr_link_local_ports[(tmp /8)];
+    data = router->vr_link_local_ports[(tmp / 8)];
     if (data & (1 << (tmp % 8)))
         return true;
 
@@ -117,7 +117,6 @@ static void
 vr_clear_link_local_port(struct vrouter *router, int family,
                        int proto, int port)
 {
-
     unsigned char *data;
     unsigned int tmp;
 
@@ -135,15 +134,16 @@ vr_clear_link_local_port(struct vrouter *router, int family,
     if (proto == VR_IP_PROTO_UDP)
         tmp += (router->vr_link_local_ports_size * 8 / 2);
 
-    data = &router->vr_link_local_ports[(tmp /8)];
+    data = &router->vr_link_local_ports[(tmp / 8)];
     *data &= (~(1 << (tmp % 8)));
+
+    return;
 }
 
 static void
 vr_set_link_local_port(struct vrouter *router, int family,
                        int proto, int port)
 {
-
     unsigned char *data;
     unsigned int tmp;
 
@@ -161,8 +161,10 @@ vr_set_link_local_port(struct vrouter *router, int family,
     if (proto == VR_IP_PROTO_UDP)
         tmp += (router->vr_link_local_ports_size * 8 / 2);
 
-    data = &router->vr_link_local_ports[tmp /8];
+    data = &router->vr_link_local_ports[tmp / 8];
     *data |= (1 << (tmp % 8));
+
+    return;
 }
 
 static void
@@ -200,7 +202,13 @@ vr_reset_flow_entry(struct vrouter *router, struct vr_flow_entry *fe,
         unsigned int index)
 {
     memset(&fe->fe_stats, 0, sizeof(fe->fe_stats));
-    memset(&fe->fe_hold_list, 0, sizeof(fe->fe_hold_list));;
+
+    if (fe->fe_hold_list) {
+        vr_printf("vrouter: Potential memory leak @ %s:%d\n",
+                __FILE__, __LINE__);
+    }
+    fe->fe_hold_list = NULL;
+
     memset(&fe->fe_key, 0, sizeof(fe->fe_key));
 
     vr_flow_reset_mirror(router, fe, index);
@@ -358,6 +366,7 @@ vr_flow_queue_free_defer(struct vr_flow_md *flmd, struct vr_flow_queue *vfq)
 
     vdd->vdd_data = (void *)vfq;
     vr_defer(flmd->flmd_router, vr_flow_queue_free, (void *)vdd);
+    flmd->flmd_defer_data = NULL;
 
     return;
 }
@@ -1330,6 +1339,11 @@ vr_flow_flush(void *arg)
     } 
 
 exit_flush:
+    if (flmd->flmd_defer_data) {
+        vr_put_defer_data(flmd->flmd_defer_data);
+        flmd->flmd_defer_data = NULL;
+    }
+
     vr_free(flmd);
 
     return;
@@ -1896,6 +1910,8 @@ vr_link_local_ports_reset(struct vrouter *router)
         memset(router->vr_link_local_ports,
                0, router->vr_link_local_ports_size);
     }
+
+    return;
 }
 
 
@@ -1908,6 +1924,8 @@ vr_link_local_ports_exit(struct vrouter *router)
         router->vr_link_local_ports = NULL;
         router->vr_link_local_ports_size = 0;
     }
+
+    return;
 }
 
 int

dp-core/vr_mpls.c

@@ -15,7 +15,7 @@
 struct vr_nexthop *
 __vrouter_get_label(struct vrouter *router, unsigned int label)
 {
-    if (!router || label > router->vr_max_labels)
+    if (!router || label >= router->vr_max_labels)
         return NULL;
 
     return router->vr_ilm[label];
@@ -29,6 +29,17 @@ vrouter_get_label(unsigned int rid, unsigned int label)
     return __vrouter_get_label(router, label);
 }
 
+static void
+__vr_mpls_del(struct vrouter *router, unsigned int label)
+{
+    if (router->vr_ilm[label])
+        vrouter_put_nexthop(router->vr_ilm[label]);
+
+    router->vr_ilm[label] = NULL;
+
+    return;
+}
+
 int
 vr_mpls_del(vr_mpls_req *req)
 {
@@ -41,16 +52,12 @@ vr_mpls_del(vr_mpls_req *req)
         goto generate_resp;
     }
 
-    if (req->mr_label > (int)router->vr_max_labels) {
+    if ((unsigned int)req->mr_label >= router->vr_max_labels) {
         ret = -EINVAL;
         goto generate_resp;
     }
 
-    if (router->vr_ilm[req->mr_label])
-        vrouter_put_nexthop(router->vr_ilm[req->mr_label]);
-
-    router->vr_ilm[req->mr_label] = NULL;
-
+    __vr_mpls_del(router, (unsigned int)req->mr_label);
 generate_resp:
     vr_send_response(ret);
 
@@ -70,11 +77,13 @@ vr_mpls_add(vr_mpls_req *req)
         goto generate_resp;
     }
 
-    if ((unsigned int)req->mr_label > router->vr_max_labels) {
+    if ((unsigned int)req->mr_label >= router->vr_max_labels) {
         ret = -EINVAL;
         goto generate_resp;
     }
 
+    __vr_mpls_del(router, (unsigned int)req->mr_label);
+
     nh = vrouter_get_nexthop(req->mr_rid, req->mr_nhid);
     if (!nh)  {
         ret = -EINVAL;
@@ -145,17 +154,26 @@ vr_mpls_get(vr_mpls_req *req)
     struct vrouter *router;
 
     router = vrouter_get(req->mr_rid);
-    if (!router || req->mr_label > (int)router->vr_max_labels) {
+    if (!router) {
         ret = -ENODEV;
-    } else {
-        nh = vrouter_get_label(req->mr_rid, req->mr_label);
-        if (!nh)
-            ret = -ENOENT;
+        goto generate_response;
     }
 
-    if (!ret)
-        vr_mpls_make_req(req, nh, req->mr_label);
-    else
+    if ((unsigned int)req->mr_label >= router->vr_max_labels) {
+        ret = -EINVAL;
+        goto generate_response;
+    }
+
+    nh = vrouter_get_label(req->mr_rid, req->mr_label);
+    if (!nh) {
+        ret = -ENOENT;
+        goto generate_response;
+    }
+
+    vr_mpls_make_req(req, nh, req->mr_label);
+
+generate_response:
+    if (ret)
         req = NULL;
 
     vr_message_response(VR_MPLS_OBJECT_ID, req, ret);

dp-core/vr_nexthop.c

@@ -88,7 +88,7 @@ vrouter_add_nexthop(struct vr_nexthop *nh)
 {
     struct vrouter *router = vrouter_get(nh->nh_rid);
 
-    if (!router || nh->nh_id > router->vr_max_nexthops)
+    if (!router || nh->nh_id >= router->vr_max_nexthops)
         return -EINVAL;
 
     /*
@@ -109,7 +109,7 @@ nh_del(struct vr_nexthop *nh)
 {
     struct vrouter *router = vrouter_get(nh->nh_rid);
 
-    if (!router || nh->nh_id > router->vr_max_nexthops)
+    if (!router || nh->nh_id >= router->vr_max_nexthops)
         return; 
 
     if (router->vr_nexthops[nh->nh_id]) {