RE: [PATCH 2/6] ip_frag: discard datagrams with overlapping fragments

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, 16 June 2026 23.06
> 
> Existing code does not handle overlapping fragments.
> 
> RFC 8200 (IPv6) requires that on overlap all reassembly is abandoned
> andall received fragments are dropped. RFC 791 (IPv4) originally called
> fortrimming and rewriting, but Linux discards for IPv4 as well, since
> overlap has no legitimate use and is a known attack vector.
> 
> Depends on the duplicate-tolerance change so that an exact duplicate is
> dropped on its own rather than discarding the whole datagram.
> 
> Fixes: cc8f4d020c0b ("examples/ip_reassembly: initial import")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/ip_frag/ip_frag_internal.c | 34 ++++++++++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/ip_frag/ip_frag_internal.c
> b/lib/ip_frag/ip_frag_internal.c
> index 9a03ef995a..2505314a29 100644
> --- a/lib/ip_frag/ip_frag_internal.c
> +++ b/lib/ip_frag/ip_frag_internal.c
> @@ -92,16 +92,34 @@ ip_frag_process(struct ip_frag_pkt *fp, struct
> rte_ip_frag_death_row *dr,
>  	uint32_t i, idx;
> 
>  	/*
> -	 * Discard an exact duplicate fragment. If a previously stored
> fragment
> -	 * already covers the same offset and length, this fragment
> carries no
> -	 * new data. Reassembly is tolerant of duplicates (RFC 791), so
> drop
> -	 * only this mbuf and keep the reassembly entry intact rather
> than
> -	 * treating it as an error. Fragments overlapping an existing one
> with
> -	 * different bounds are not handled here.
> +	 * Scan the fragments already collected for this datagram before
> +	 * storing the new one. The stored set is kept free of duplicates
> and
> +	 * overlaps, so a single pass is sufficient.
>  	 */
>  	for (i = 0; i != fp->last_idx; i++) {
> -		if (fp->frags[i].mb != NULL && fp->frags[i].ofs == ofs &&
> -				fp->frags[i].len == len) {
> +		if (fp->frags[i].mb == NULL)
> +			continue;
> +
> +		/*
> +		 * Exact duplicate: carries no new data. Reassembly
> tolerates
> +		 * duplicates (RFC 791), so drop only this mbuf and keep
> the
> +		 * entry.
> +		 */
> +		if (fp->frags[i].ofs == ofs && fp->frags[i].len == len) {
> +			IP_FRAG_MBUF2DR(dr, mb);
> +			return NULL;
> +		}
> +
> +		/*
> +		 * Overlap with an existing fragment. Per RFC 8200 section
> 4.5
> +		 * (and RFC 5722) the datagram must be discarded; the same
> is
> +		 * applied to IPv4. Free all collected fragments, drop this
> one,
> +		 * and invalidate the entry.
> +		 */
> +		if (ofs < fp->frags[i].ofs + fp->frags[i].len &&
> +				fp->frags[i].ofs < ofs + len) {

This only catches fragments that are smaller than existing fragments, i.e. fit within one of the existing fragments.
It should be:
if ((ofs >= fp->frags[i].ofs &&
		ofs < fp->frags[i].ofs + fp->frags[i].len) ||
		(ofs + len >= fp->frags[i].ofs &&
		ofs + len < fp->frags[i].ofs + fp->frags[i].len)) {

> +			ip_frag_free(fp, dr);
> +			ip_frag_key_invalidate(&fp->key);
>  			IP_FRAG_MBUF2DR(dr, mb);
>  			return NULL;
>  		}
> --
> 2.53.0