|
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 <ja@xxxxxx>
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 <ja@xxxxxx>
|
