|WARNING| [v1] fib6: fix sign conversion of tbl8 indexes

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Oct 22 02:39:30 CEST 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171819

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-22

# DPDK Patch Review

## ERRORS

**Integer multiply without widening cast (line 333 in original code, line 334 after patch)**

```c
p = get_tbl_p_by_idx(dp->tbl8, (tbl8_idx *
	TRIE_TBL8_GRP_NUM_ENT) + *ip_part, dp->nh_sz);
```

The expression `tbl8_idx * TRIE_TBL8_GRP_NUM_ENT` is performed at `int` width before the result is passed to `get_tbl_p_by_idx()`. When `tbl8_idx` approaches the upper range of valid `int` values, this multiplication can overflow before being widened to the index type expected by the function.

The patch correctly widens `tbl8_idx` to `uint64_t` in two shift operations at lines 332 and 363, but this multiply at line 363 (in the patched version) still needs the same treatment.

**Suggested fix:**

```c
p = get_tbl_p_by_idx(dp->tbl8, ((uint64_t)tbl8_idx *
	TRIE_TBL8_GRP_NUM_ENT) + *ip_part, dp->nh_sz);
```

---

## Analysis

The patch addresses a real correctness bug: sign extension of a 32-bit value with the high bit set, when used as a 64-bit index, produces an invalid pointer. The crash described in the commit message (SIGSEGV at tbl8 group 0x800001) and the use of separate signed/unsigned variables for error handling versus indexing are both correct approaches.

The two casts to `uint64_t` before left-shifting `tbl8_idx` (lines 332 and 363) correctly prevent the shift from being performed in `int` width and overflowing.

However, the multiplication of `tbl8_idx` by `TRIE_TBL8_GRP_NUM_ENT` at line 363 has the same risk: if `tbl8_idx` is large enough (and the patch context shows it can reach at least 0x800001), the product can overflow `int` before being widened to the type expected by `get_tbl_p_by_idx()`. The two issues (shift overflow and multiply overflow) stem from the same root cause and the patch fixes one but not the other.

Review-Result: ERROR


More information about the test-report mailing list