[PATCH] net: fix GTP Tunnel parse out-of-bounds read

[Andrew Rybchenko]

On 6/1/26 3:59 PM, Thomas Monjalon wrote:
> Any comment about this fix?
> 
> 
> 09/04/2026 18:15, Stephen Hemminger:
>> If packet is fragmented across multiple mbufs or the packet
>> has only GTP header the code would reference outside
>> the incoming mbuf.
>>
>> Send GTP packet:
>> - Valid GTP header (8 bytes)
>> - msg_type = 0xff
>> - e=1, s=1, pn=1 (sets gtp_len = 12)
>> - Total packet size = 10 bytes
>>
>> Read at gh + 12 accesses 2 bytes beyond packet end.
>>
>> The fix is to use rte_pktmbuf_read in a manner similar
>> to the read of the GTP header.
>>
>> Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")
>> Cc: stable at dpdk.org
>>
>> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>

The overall idea LGTM. However one nit in the code.

Reviewed-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>

>> ---
>>   lib/net/rte_net.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> 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;
>>   		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;

l3_hdr is confusing here. It sounds like a complete layer 3 header, but
it is just one first byte. May be it would be useful to highlight that
it is an inner Layer 3 header.

E.g. inner_l3_hdr_byte (I'm not fully happy with the name too, but
I hope idea is clear)

>> +			uint8_t l3_copy, ip_ver;
>> +
>> +			l3_hdr = rte_pktmbuf_read(m, *off, sizeof(*l3_hdr), &l3_copy);

sizeof(l3_copy) would be safer here.

>> +			if (unlikely(l3_hdr == NULL))
>> +				return 0;
>> +
>> +			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;
>>   		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)
>>
> 
> 
> 
> 
>