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

Julian Anastasov <[email protected]> Mon, 6 Jul 2026 20:49:29 +0300 (EEST)
Newsgroups org.kernel.vger.lvs-devel,org.kernel.vger.netfilter-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]>