From 8dbc800f42797105b53fd385eb57ac30bc372d7d Mon Sep 17 00:00:00 2001
From: Stephen Worley <sworley@cumulusnetworks.com>
Date: Thu, 25 Jul 2019 13:27:59 -0400
Subject: [PATCH] zebra: Prevent duplication and overflow in nhe2grp

The kernel does not allow duplicate IDs in the same group, but
we are perfectly find with it internally if two different
nexthops resolve the the same nexthop (default route for instance).
So, we have to handle this when we get ready to install.

Further, pass the max group size in the arguments to ensure we
don't overflow. Don't actually think this is possible due to
multipath checking in nexthop_active_update() but better to be
safe.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
---
 zebra/zebra_dplane.c |  4 ++--
 zebra/zebra_nhg.c    | 26 +++++++++++++++++++++-----
 zebra/zebra_nhg.h    |  4 ++--
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 2346cb7f0db0..e4002252d3a1 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -1589,8 +1589,8 @@ static int dplane_ctx_nexthop_init(struct zebra_dplane_ctx *ctx,
 	/* If its a group, convert it to a grp array of ids */
 	if (!zebra_nhg_depends_is_empty(nhe)
 	    && !CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_RECURSIVE))
-		ctx->u.rinfo.nhe.nh_grp_count =
-			zebra_nhg_nhe2grp(ctx->u.rinfo.nhe.nh_grp, nhe);
+		ctx->u.rinfo.nhe.nh_grp_count = zebra_nhg_nhe2grp(
+			ctx->u.rinfo.nhe.nh_grp, nhe, MULTIPATH_NUM);
 
 	zns = ((struct zebra_vrf *)vrf_info_lookup(nhe->vrf_id))->zns;
 
diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index 24eae603f2cf..870f4afb2786 100644
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -1444,13 +1444,16 @@ int zebra_nhg_re_update_ref(struct route_entry *re, struct nhg_hash_entry *new)
 }
 
 /* Convert a nhe into a group array */
-uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe)
+uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe,
+			  int max_num)
 {
 	struct nhg_connected *rb_node_dep = NULL;
 	struct nhg_hash_entry *depend = NULL;
 	uint8_t i = 0;
 
 	frr_each (nhg_connected_tree, &nhe->nhg_depends, rb_node_dep) {
+		bool duplicate = false;
+
 		depend = rb_node_dep->nhe;
 
 		/*
@@ -1467,11 +1470,24 @@ uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe)
 			}
 		}
 
-		grp[i].id = depend->id;
-		/* We aren't using weights for anything right now */
-		grp[i].weight = 0;
-		i++;
+		/* Check for duplicate IDs, kernel doesn't like that */
+		for (int j = 0; j < i; j++) {
+			if (depend->id == grp[j].id)
+				duplicate = true;
+		}
+
+		if (!duplicate) {
+			grp[i].id = depend->id;
+			/* We aren't using weights for anything right now */
+			grp[i].weight = 0;
+			i++;
+		}
+
+		if (i >= max_num)
+			goto done;
 	}
+
+done:
 	return i;
 }
 
diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h
index 0be821267ef8..e06d415b28ec 100644
--- a/zebra/zebra_nhg.h
+++ b/zebra/zebra_nhg.h
@@ -236,8 +236,8 @@ extern int nexthop_active_update(struct route_node *rn, struct route_entry *re);
 extern int zebra_nhg_re_update_ref(struct route_entry *re,
 				   struct nhg_hash_entry *nhe);
 
-extern uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp,
-				 struct nhg_hash_entry *nhe);
+extern uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe,
+				 int size);
 
 void zebra_nhg_install_kernel(struct nhg_hash_entry *nhe);
 void zebra_nhg_uninstall_kernel(struct nhg_hash_entry *nhe);