From: Robert Sanford <[email protected]>
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: c3e12e0f03 ("fib: add dataplane algorithm for IPv6")
Cc: [email protected]
Signed-off-by: Robert Sanford <[email protected]>
---
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
