Re: [PATCH net v2 2/2] net: ipv6: Clamp to IP6_MAX_MTU in ip6_dst_mtu_maybe_forward

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Alice Mikityanska wrote:
> From: Alice Mikityanska <alice@isovalent.com>
> 
> Commit 427faee167bc ("net: ipv6: introduce ip6_dst_mtu_maybe_forward")
> dropped the IP6_MAX_MTU clamp that used to be present in ip6_mtu(). A
> similar IPv4 commit ac6627a28dbf ("net: ipv4: Consolidate ipv4_mtu and
> ip_dst_mtu_maybe_forward") preserves the IP_MAX_MTU clamp.
> 
> Restore the upper bound in the IPv6 flow to avoid potential 16-bit
> overflows in forwarding paths.
> 
> Fixes: 427faee167bc ("net: ipv6: introduce ip6_dst_mtu_maybe_forward")
> Signed-off-by: Alice Mikityanska <alice@isovalent.com>
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> ---
>  include/net/ip6_route.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index 09ffe0f13ce7..fb59a5885faa 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -382,6 +382,8 @@ static inline unsigned int ip6_dst_mtu_maybe_forward(const struct dst_entry *dst
>  	rcu_read_unlock();
>  
>  out:
> +	mtu = min_t(unsigned int, mtu, IP6_MAX_MTU);
> +
>  	return mtu - lwtunnel_headroom(dst->lwtstate, mtu);
>  }

It appears IPv4 only clamps device MTU, not route MTU:

        mtu = dst_metric_raw(dst, RTAX_MTU);
        if (!mtu)
                mtu = min(READ_ONCE(dst->dev->mtu), IP_MAX_MTU);

I don't think that was necessarily intentional. Perhaps route MTU
itself is already bounds checked on configuration. The device MTU
min() was added after a syzbot report, in commit c780a049f9b.

Current IPv6 proposal is arguably more robust, covering both. There
just remains a difference between IPv4 and IPv6 code paths.