On Tue, 15 Oct 2024 10:21:33 +0200 <[email protected]> wrote:
> From: Vignesh PS <[email protected]> > > Add support to ip_frag library to perform IPv6 reassembly > when extension headers are present before the fragment > extension in the packet. > > Signed-off-by: Vignesh PS <[email protected]> > --- > .mailmap | 1 + > app/test/test_reassembly_perf.c | 163 +++++++++++++++++++----------- > lib/ip_frag/ip_frag_common.h | 4 + > lib/ip_frag/ip_reassembly.h | 2 + > lib/ip_frag/rte_ipv6_reassembly.c | 75 ++++++++++++-- > 5 files changed, 179 insertions(+), 66 deletions(-) This patch was never reviewed in detail. AI review found some issues, it would need changes. ## Patch Feedback Summary ### Critical Bug — Fix Required **NULL dereference when first fragment arrives last** (`rte_ipv6_reassembly.c`) The code sets `fp->next_proto` *after* calling `ip_frag_process()`, but `ip_frag_process()` can immediately trigger `ipv6_frag_reassemble()` before returning — which dereferences `fp->next_proto`. Since `ip_frag_reset()` initializes it to NULL, any flow where the first fragment arrives last will crash. **Fix**: Move the `fp->next_proto` / `fp->exts_len` assignment block to *before* the `ip_frag_process()` call. --- ### Security Bug — Fix Required **No bounds check in `ip_frag_get_last_exthdr()`** (`rte_ipv6_reassembly.c`) The loop advances through extension headers using `ext_len` values read from packet data, with no check that the accumulated `total_len` stays within the packet's actual payload length. A crafted packet with malformed extension headers can walk the pointer off the end of the mbuf's data buffer (out-of-bounds read). Add a check that `total_len + ext_len <= ip_hdr->payload_len` on each iteration. --- ### Minor Issues - **Typo in comment**: `"or th next header"` → `"or the next header"` - **`ip_frag_get_last_exthdr` return type**: Returns `int` but accumulates into `uint32_t`; large crafted extension stacks could produce a false negative error return. Consider returning `int32_t` with a documented cap, or restructuring to use an out-parameter.
