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

[Medvedkin, Vladimir]

Hi Robert,

Thanks for the patch

I see two ways to fix this: either add a check at creation time so that 
no more than 2^23 tbl8 groups are accepted (which should generally be 
fine, although there may be exceptions, hence option 2), or widen the 
variables holding tbl8 indexes, as this patch does.

This patch fixes the issue in the "roughly 630,000 to 650,000 routes" 
range, but not completely. It only gains a single bit, so the problem 
reappears at tbl8 group 2^24. With /128 routes as in your test, that is 
at about 1.3M routes, and this time lookups silently return wrong next 
hops instead of crashing. I'd encourage you to widen these indexes to 
uint64_t, not only in build_common_root(), but also in get_idx(), 
get_tbl_pos() and get_tbl_val_by_idx().

On 10/21/2026 7:18 PM, Robert Sanford wrote:
> From: Robert Sanford <rsanford at 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 at dpdk.org
>
> Signed-off-by: Robert Sanford <rsanford at 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);

-- 
Regards,
Vladimir