[PATCH v2] fib6: fix sign conversion of tbl8 indexes

[Robert Sanford <rsanford2@gmail.com>]

From: Robert Sanford <rsanford@akamai.com>

In build_common_root(), get_idx() returns a uint32_t, but the result
was stored in an int. Once the tbl8 group index reaches 2^23
(prev_idx * 256 reaches 2^31), the value has its high bit set and
goes negative. It is then sign-extended when converted to the
uint64_t index passed to get_tbl_val_by_idx() and
get_tbl_p_by_idx(), which yields a bogus table pointer.

This was hit in practice: a modified fib6_perf_autotest with 750,000
random /128 routes crashed with SIGSEGV in install_to_dp(), where
build_common_root() is inlined, when tbl8 group 0x800001 was
allocated (after roughly 630,000 to 650,000 routes). The faulting
store went to the table base plus the sign-extended index 0x800000a4
(a negative int), which wrapped to an unmapped address. With this
change the same test runs to completion.

Make idx and prev_idx unsigned, and use a separate signed tbl8_idx
for the tbl8_alloc() result, which returns a negative value on error.

Also cast tbl8_idx to uint64_t before shifting it left, in
build_common_root() and in write_edge(). The shift was done in int,
which can overflow before the result is widened.

The problem is also visible at compile time: GCC's -Wsign-conversion
reports the original get_idx() assignment, without having to build a
table large enough to hit it.

Fixes: c3e12e0f0354 ("fib: add dataplane algorithm for IPv6")
Cc: stable@dpdk.org

Signed-off-by: Robert Sanford <rsanford@akamai.com>
---
v2: use a 12-character sha in the Fixes tag.

 lib/fib/trie.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/fib/trie.c b/lib/fib/trie.c
index 187360d1c8..95f79e26a5 100644
--- a/lib/fib/trie.c
+++ b/lib/fib/trie.c
@@ -317,7 +317,8 @@ build_common_root(struct rte_trie_tbl *dp, const struct rte_ipv6_addr *ip,
 	void *tbl_ptr = NULL;
 	uint64_t *cur_tbl;
 	uint64_t val;
-	int i, j, idx, prev_idx = 0;
+	uint32_t idx, prev_idx = 0;
+	int i, j, tbl8_idx;
 
 	cur_tbl = dp->tbl24;
 	for (i = 3, j = 0; i <= common_bytes; i++) {
@@ -325,12 +326,12 @@ build_common_root(struct rte_trie_tbl *dp, const struct rte_ipv6_addr *ip,
 		val = get_tbl_val_by_idx(cur_tbl, idx, dp->nh_sz);
 		tbl_ptr = get_tbl_p_by_idx(cur_tbl, idx, dp->nh_sz);
 		if ((val & TRIE_EXT_ENT) != TRIE_EXT_ENT) {
-			idx = tbl8_alloc(dp, val);
-			if (unlikely(idx < 0))
-				return idx;
-			write_to_dp(tbl_ptr, (idx << 1) |
+			tbl8_idx = tbl8_alloc(dp, val);
+			if (unlikely(tbl8_idx < 0))
+				return tbl8_idx;
+			write_to_dp(tbl_ptr, ((uint64_t)tbl8_idx << 1) |
 				TRIE_EXT_ENT, dp->nh_sz, 1);
-			prev_idx = idx;
+			prev_idx = (uint32_t)tbl8_idx;
 		} else
 			prev_idx = val >> 1;
 
@@ -359,7 +360,7 @@ write_edge(struct rte_trie_tbl *dp, const uint8_t *ip_part, uint64_t next_hop,
 			tbl8_idx = tbl8_alloc(dp, val);
 			if (tbl8_idx < 0)
 				return tbl8_idx;
-			val = (tbl8_idx << 1)|TRIE_EXT_ENT;
+			val = ((uint64_t)tbl8_idx << 1) | TRIE_EXT_ENT;
 		}
 		p = get_tbl_p_by_idx(dp->tbl8, (tbl8_idx *
 			TRIE_TBL8_GRP_NUM_ENT) + *ip_part, dp->nh_sz);
-- 
2.34.1