On Wed, 22 Apr 2026 15:36:16 +0200 Robin Jarry <[email protected]> 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 <[email protected]> > --- AI review feedback spotted some things I didn't On Wed, 22 Apr 2026 15:36:16 +0200 Robin Jarry <[email protected]> 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.
