Re: [PATCH nf] ipvs: ensure inner headers in ICMP errors are in headroom

Julian Anastasov <[email protected]>
Newsgroups gmane.comp.security.firewalls.netfilter.devel,gmane.comp.linux.lvs.devel
Message-ID <[email protected]>
	Hello,

On Mon, 6 Jul 2026, Julian Anastasov wrote:

> Sashiko points out that after stripping the outer headers
> with pskb_pull() we should ensure the inner IP headers
> in ICMP errors from tunnels are present in the skb headroom
> for functions like ipv4_update_pmtu(), icmp_send() and
> IP_VS_DBG().
> 
> Also, add more checks for the length of the inner headers.
> 
> Fixes: f2edb9f7706d ("ipvs: implement passive PMTUD for IPIP packets")
> Link: https://sashiko.dev/#/patchset/20260702073430.67680-1-zhaoyz24%40mails.tsinghua.edu.cn
> Signed-off-by: Julian Anastasov <[email protected]>

	Ignore this, will send v2 with more checks...
        
pw-bot: changes-requested

> ---
>  net/netfilter/ipvs/ip_vs_core.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index 906f2c361676..f332ba422a65 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1767,6 +1767,7 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  	bool tunnel, new_cp = false;
>  	union nf_inet_addr *raddr;
>  	char *outer_proto = "IPIP";
> +	unsigned int hlen_ipip = 0;
>  	int ulen = 0;
>  
>  	*related = 1;
> @@ -1822,9 +1823,10 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  		/* Only for known tunnel */
>  		if (!dest || dest->tun_type != IP_VS_CONN_F_TUNNEL_TYPE_IPIP)
>  			return NF_ACCEPT;
> -		offset += cih->ihl * 4;
> +		hlen_ipip = cih->ihl * 4;
> +		offset += hlen_ipip;
>  		cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
> -		if (cih == NULL)
> +		if (!(cih && cih->version == 4 && cih->ihl >= 5))
>  			return NF_ACCEPT; /* The packet looks wrong, ignore */
>  		tunnel = true;
>  	} else if ((cih->protocol == IPPROTO_UDP ||	/* Can be UDP encap */
> @@ -1836,7 +1838,8 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  		/* Non-first fragment has no UDP/GRE header */
>  		if (unlikely(cih->frag_off & htons(IP_OFFSET)))
>  			return NF_ACCEPT;
> -		offset2 = offset + cih->ihl * 4;
> +		hlen_ipip = cih->ihl * 4;
> +		offset2 = offset + hlen_ipip;
>  		if (cih->protocol == IPPROTO_UDP) {
>  			ulen = ipvs_udp_decap(ipvs, skb, offset2, AF_INET,
>  					      raddr, &iproto);
> @@ -1905,6 +1908,7 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  	}
>  
>  	if (tunnel) {
> +		unsigned int hlen_orig = cih->ihl * 4;
>  		__be32 info = ic->un.gateway;
>  		__u8 type = ic->type;
>  		__u8 code = ic->code;
> @@ -1921,6 +1925,9 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  				goto ignore_tunnel;
>  			offset2 -= ihl + sizeof(_icmph);
>  			skb_reset_network_header(skb);
> +			/* Ensure the IP header is present in headroom */
> +			if (!pskb_may_pull(skb, hlen_ipip))
> +				goto ignore_tunnel;
>  			IP_VS_DBG(12, "ICMP for %s %pI4->%pI4: mtu=%u\n",
>  				  outer_proto, &ip_hdr(skb)->saddr,
>  				  &ip_hdr(skb)->daddr, mtu);
> @@ -1936,8 +1943,8 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  				if (dest_dst)
>  					mtu = dst_mtu(dest_dst->dst_cache);
>  			}
> -			if (mtu > 68 + sizeof(struct iphdr) + ulen)
> -				mtu -= sizeof(struct iphdr) + ulen;
> +			if (mtu > 68 + hlen_ipip + ulen)
> +				mtu -= hlen_ipip + ulen;
>  			info = htonl(mtu);
>  		}
>  		/* Strip outer IP, ICMP and IPIP/UDP/GRE, go to IP header of
> @@ -1946,6 +1953,9 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  		if (pskb_pull(skb, offset2) == NULL)
>  			goto ignore_tunnel;
>  		skb_reset_network_header(skb);
> +		/* Ensure the IP header is present in headroom */
> +		if (!pskb_may_pull(skb, hlen_orig))
> +			goto ignore_tunnel;
>  		IP_VS_DBG(12, "Sending ICMP for %pI4->%pI4: t=%u, c=%u, i=%u\n",
>  			&ip_hdr(skb)->saddr, &ip_hdr(skb)->daddr,
>  			type, code, ntohl(info));
> -- 
> 2.55.0

Regards

--
Julian Anastasov <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.