DPDK ip_frag security analyis

Subject: Security analysis of lib/ip_frag (fragmentation & reassembly)

Hi,

Below is a security-focused review of the DPDK ip_frag library covering ip_frag_common.h, ip_frag_internal.c, ip_reassembly.h, rte_ip_frag.h, rte_ip_frag_common.c, rte_ipv4_fragmentation.c, rte_ipv4_reassembly.c, rte_ipv6_fragmentation.c, and rte_ipv6_reassembly.c.

Findings are grouped by severity.

ERRORS
======

1. TAILQ_FOREACH modification during iteration in
rte_ip_frag_table_del_expired_entries()

File: rte_ip_frag_common.c, lines 142-151

TAILQ_FOREACH(fp, &tbl->lru, lru) iterates the LRU list while ip_frag_tbl_del() calls TAILQ_REMOVE() on the current element inside the loop body. TAILQ_FOREACH reads fp->lru.tqe_next in the loop-advance step *after* the body executes. After TAILQ_REMOVE, the removed element's tqe_next is stale.

On glibc/BSD this happens to work because TAILQ_REMOVE does not zero tqe_next, but this violates the TAILQ API contract. If the implementation ever changes, or if a debug TAILQ that poisons removed entries is used, this becomes a use-after-remove bug.

Fix: use TAILQ_FOREACH_SAFE, or save TAILQ_NEXT(fp, lru) before calling ip_frag_tbl_del().

2. No overlapping fragment detection (teardrop-class attacks, RFC 5722 violation for IPv6)

File: ip_frag_internal.c, ip_frag_process()

ip_frag_process() does not check whether a newly arrived fragment overlaps with any previously stored fragment. It only detects duplicate first/last fragments. For middle fragments, it blindly increments last_idx and stores the fragment.

The reassembly functions happen to fail on overlapping fragments because the offset-matching chain walk cannot find a match, so the hole-detection fires. The mitigation is accidental rather than intentional.

For IPv6, RFC 5722 mandates that overlapping fragments MUST cause the entire datagram to be silently dropped. The current code does eventually drop them, but only after accumulating fragments and burning table slots until timeout. An attacker can exhaust fragment table entries by sending overlapping fragments.

Fix: check new fragment offset/length against existing entries in frags[] before storing. For IPv6, drop immediately per RFC 5722.

3. Death row buffer overflow -- IP_FRAG_MBUF2DR has no bounds check

File: ip_frag_common.h, line 38

The IP_FRAG_MBUF2DR(dr, mb) macro writes to dr->row[dr->cnt++] with no bounds check. Similarly, ip_frag_free() adds up to IP_MAX_FRAG_NUM (8) entries without checking capacity.

The death row is sized for RTE_IP_FRAG_DEATH_ROW_LEN * (MAX_FRAG + 1) = 288 entries. The del_expired_entries() function does check capacity, but the hot-path reassembly functions call ip_frag_find() -> ip_frag_tbl_del() and ip_frag_process() -> ip_frag_free() + IP_FRAG_MBUF2DR() without any capacity check.

If an application calls reassembly in a tight loop without draining the death row between calls, dr->cnt can exceed RTE_IP_FRAG_DEATH_ROW_MBUF_LEN, causing an out-of-bounds write. The death row is typically stack-allocated, so this is a potential stack buffer overflow.

ip_frag_find() can evict a stale entry AND an LRU entry in a single call (up to 16 mbufs), then ip_frag_process() can add 9 more on error.A burst of 32 bad fragments hitting different stale entries could overflow.

Fix: add a bounds check in IP_FRAG_MBUF2DR and ip_frag_free(), or document the maximum number of reassembly calls permitted between death row drains.

WARNINGS
========

4. IPv6 move_len underflow in ipv6_frag_reassemble()

File: rte_ipv6_reassembly.c, line 104

   move_len = m->l2_len + m->l3_len - sizeof(*frag_hdr);

move_len is uint32_t. If a buggy caller sets l2_len + l3_len < 8, move_len wraps to ~4 billion. The subsequent ip_frag_memmove() would attempt to copy gigabytes of data, causing a segfault or memory corruption.

The API documents that callers must set l2_len/l3_len correctly, and for a valid IPv6 fragment l3_len alone should be >= 48 bytes. But there is no defensive validation.

Fix: add a sanity check that l2_len + l3_len >= sizeof(*frag_hdr) before computing move_len.

5. rte_ipv6_frag_get_ipv6_fragment_header() only inspects the first extension header

File: rte_ip_frag.h, line 143

This function checks only hdr->proto == IPPROTO_FRAGMENT. If any other extension header (hop-by-hop, routing, destination options) precedes the fragment header, this function returns NULL and the packet is treated as non-fragmented.

An attacker can evade reassembly by inserting a preceding extension header, causing fragments to be forwarded as-is. This can bypass firewall rules that rely on seeing reassembled traffic.

The limitation is documented but is a security-relevant design gap for applications using this library.

Fix: walk the extension header chain to find the fragment header, or clearly document the security implications in the API header.

INFORMATIONAL
=============

6. Hash collision DoS via fixed seed

Both ipv4_frag_hash() and ipv6_frag_hash() use CRC32 (x86/ARM) or jhash with a fixed, publicly known prime seed (0xeaad8405). An attacker who can send crafted IP fragments can precompute hash collisions, causing all fragments to land in the same bucket. After bucket_entries concurrent flows collide, new flows are dropped.

Fix: randomize the hash seed at table creation time.

7. max_cycles + fp->start wraparound

The timeout comparison (max_cycles + fp->start < tms) at multiple locations uses uint64_t arithmetic. If the sum wraps past 2^64, entries appear timed out when they are not. With rdtsc-based timestamps this requires running for hundreds of years, so it is not practically exploitable. Worth noting for alternative time sources with faster-incrementing values.

8. No total reassembled size limit before table insertion

The library accepts and stores fragments up to IP_MAX_FRAG_NUM per datagram, but there is no explicit limit on total reassembled size. An attacker can claim total_size = 65535 via a crafted last fragment, then send middle fragments that consume table entries and death row capacity during the attack window, even though reassembly will ultimately fail when frag_size exceeds total_size.

Quoted 2 lines

Fix: use TAILQ_FOREACH_SAFE, or save TAILQ_NEXT(fp, lru) before calling ip_frag_tbl_del().

ACK, that looks like a valid one to me.

Quoted 9 lines

6. Hash collision DoS via fixed seed

Both ipv4_frag_hash() and ipv6_frag_hash() use CRC32 (x86/ARM) or jhash with a fixed, publicly known prime seed (0xeaad8405). An attacker who can send crafted IP fragments can precompute hash collisions, causing all fragments to land in the same bucket. After bucket_entries concurrent flows collide, new flows are dropped.

Quoted 1 line

Fix: randomize the hash seed at table creation time.

ACK, seems valid - needs to be fixed.