Re: [PATCH dpdk] net/tap: use offsets provided by rte_net_get_ptype

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 22 Apr 2026 15:36:16 +0200
Robin Jarry <rjarry@redhat.com> wrote:

> Instead of guessing what are the proper header lengths, pass
> a rte_net_hdr_lens struct to rte_net_get_ptype and use it to get the
> proper header lengths/offsets in tap_verify_csum.
> 
> Signed-off-by: Robin Jarry <rjarry@redhat.com>
> ---

AI review feedback spotted some things I didn't

On Wed, 22 Apr 2026 15:36:16 +0200
Robin Jarry <rjarry@redhat.com> wrote:

> Instead of guessing what are the proper header lengths, pass
> a rte_net_hdr_lens struct to rte_net_get_ptype and use it to get the
> proper header lengths/offsets in tap_verify_csum.

The two bounds checks this patch drops are not redundant with the new
l4_off check and need to stay in some form:

-	if (l2_len + rte_be_to_cpu_16(iph->total_length) >
-			rte_pktmbuf_data_len(mbuf))
-		return;
...
-	if (l2_len + l3_len + rte_be_to_cpu_16(iph->payload_len) >
-			rte_pktmbuf_data_len(mbuf))
-		return;

rte_ipv4_udptcp_cksum_verify() -> __rte_ipv4_udptcp_cksum() does:

	l3_len = rte_be_to_cpu_16(ipv4_hdr->total_length);
	...
	l4_len = l3_len - ip_hdr_len;
	cksum = rte_raw_cksum(l4_hdr, l4_len);

and the IPv6 variant reads payload_len the same way. Both are untrusted
values from the wire. Without the checks above, a packet whose
total_length / payload_len exceeds what was actually received causes
rte_raw_cksum() to read past the valid data - uninitialized tail room at
best, outside buf_len at worst. The IP/L4 CKSUM flags set on the mbuf
will then be computed over stale memory.

These checks were added deliberately in:

  1168a4fd193 ("net/tap: add buffer overflow checks before checksum")

Please restore equivalent validation after rte_net_get_ptype(), e.g.
check iph->total_length (IPv4) and hdr_lens->l3_len + iph->payload_len
(IPv6) against rte_pktmbuf_data_len(mbuf) - hdr_lens->l2_len before
calling the checksum verifier.

One other thing: rte_net_get_ptype() returns RTE_PTYPE_L3_IPV6_EXT when
IPv6 extensions are present. The new "else if (l3 != RTE_PTYPE_L3_IPV6)"
clause falls through to return without setting any CKSUM flag for that
case. That matches pre-patch behavior, but the comment the patch removed
was the only thing documenting it. Either accept IPv6_EXT here (l3_len
now spans the extensions, so the existing helpers work as long as the
next header is TCP/UDP) or keep the comment.

Nit: the memset(&hdr_lens, 0, ...) runs on every Rx packet even when
RX_OFFLOAD_CHECKSUM is disabled. Moving it plus the &hdr_lens argument
inside the offload branch (passing NULL otherwise) avoids that.

I prefer initialization instead of memset, less error prone and easier.