|SUCCESS| net: fix GTP Tunnel parse out-of-bounds read

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162967

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-09

Reviewing patch: net: fix GTP Tunnel parse out-of-bounds read

sha: (from patch file series.patch)
Author: Stephen Hemminger <stephen at networkplumber.org>
Subject: [PATCH] net: fix GTP Tunnel parse out-of-bounds read

This patch addresses an out-of-bounds read vulnerability in GTP tunnel
packet parsing where fragmented or malformed packets could cause the code
to access memory beyond the packet boundaries.

> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
> index 458b4814a9..da4018437b 100644
> --- a/lib/net/rte_net.c
> +++ b/lib/net/rte_net.c
> @@ -219,8 +219,7 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
>  	case RTE_GTPU_UDP_PORT: {
>  		const struct rte_gtp_hdr *gh;
>  		struct rte_gtp_hdr gh_copy;
> -		uint8_t gtp_len;
> -		uint8_t ip_ver;
> +		uint32_t gtp_len;

The change from uint8_t to uint32_t for gtp_len appears inconsistent with
the actual field size. Can gtp_len overflow a uint8_t based on the
rte_gtp_hdr structure definition? If the underlying field in the header is
smaller, would this type change introduce problems where gtp_len could
contain values that exceed what fits in the actual header field?

>  		gh = rte_pktmbuf_read(m, *off, sizeof(*gh), &gh_copy);
>  		if (unlikely(gh == NULL))
>  			return 0;
> @@ -231,9 +230,16 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
>  		 * Check message type. If message type is 0xff, it is
>  		 * a GTP data packet. If not, it is a GTP control packet
>  		 */
> +		*off += gtp_len;
>  		if (gh->msg_type == 0xff) {
> -			ip_ver = *(const uint8_t *)((const char *)gh + gtp_len);
> -			ip_ver = (ip_ver) & 0xf0;
> +			const uint8_t *l3_hdr;
> +			uint8_t l3_copy, ip_ver;
> +
> +			l3_hdr = rte_pktmbuf_read(m, *off, sizeof(*l3_hdr), &l3_copy);
> +			if (unlikely(l3_hdr == NULL))
> +				return 0;

Does this code properly handle packet boundary validation? The offset *off
was just incremented by gtp_len, but is there any verification that *off
doesn't exceed the packet length before calling rte_pktmbuf_read()? While
rte_pktmbuf_read() returns NULL on failure, should there be explicit
bounds checking here?

> +
> +			ip_ver = *l3_hdr & 0xf0;
>  			if (ip_ver == RTE_GTP_TYPE_IPV4)
>  				*proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
>  			else if (ip_ver == RTE_GTP_TYPE_IPV6)
> @@ -243,7 +249,6 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
>  		} else {
>  			*proto = 0;
>  		}
> -		*off += gtp_len;

Moving the offset increment before the l3_hdr read changes the control
flow. In the gh->msg_type != 0xff path (GTP control packets), does the
offset still get incremented correctly? The original code incremented *off
after both branches, but now it happens before the conditional. Does this
affect the offset calculation for GTP control packets?

>  		hdr_lens->inner_l2_len = gtp_len + sizeof(struct rte_udp_hdr);
>  		hdr_lens->tunnel_len = gtp_len;
>  		if (port_no == RTE_GTPC_UDP_PORT)