Re: [PATCH net v2] ipv6: fix dead half of the MTU guard in __ip6_append_data()

[Narcisa Vasile <narcisav.kernel@gmail.com>]

On Thu, Oct 29, 2026 at 10:38:57AM +0800, bo cao wrote:
> ipv6: fix dead half of the MTU guard in __ip6_append_data()
> 
>  net/ipv6/ip6_output.c | 41 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 8 deletions(-)
> 

The patchwork check indicates that two more people should be added to CC.

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 25fe0ba98b1a..ab4eee51f173 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
>  
>  static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
> @@ -1499,13 +1513,23 @@ static int __ip6_append_data(struct sock *sk,
>  		     (opt ? opt->opt_flen + opt->opt_nflen : 0) +
>  		     rt->rt6i_nfheader_len;
>  
> -	if (mtu <= fragheaderlen ||
> -	    ((mtu - fragheaderlen) & ~7) + fragheaderlen <= sizeof(struct frag_hdr))
> +	if (mtu <= fragheaderlen)
>  		goto emsgsize;
>  

It might be simpler to just contain the whole fix within __ip6_append_data()
without changing ip6_append_data_mtu().

Just factor Sashiko's high severity finding into the check
suggested by Willem. Something like this (untested):

-       if (mtu <= fragheaderlen)
+       reserved_headerlen = fragheaderlen;
+       if (!skb && !(rt->dst.flags & DST_XFRM_TUNNEL))
+               reserved_headerlen += rt->dst.header_len;
+
+       if (mtu < reserved_headerlen + sizeof(struct frag_hdr) + 8)
                goto emsgsize;

>  	maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen -
>  		     sizeof(struct frag_hdr);
>  
> +	/*
> +	 * Each non-final fragment must leave room for at least 8 bytes of
> +	 * payload after the fragment header, matching ip6_fragment()'s
> +	 * mtu < hlen + sizeof(struct frag_hdr) + 8 check.  This rejects
> +	 * MTUs that make maxfraglen underflow below fragheaderlen and
> +	 * defeat the datalen clamp, which leads to skb_over_panic() from
> +	 * skb_put().
> +	 */
> +	if (maxfraglen < fragheaderlen + sizeof(struct frag_hdr) + 8)
> +		goto emsgsize;
> +

You could then remove this maxfraglen check and its comment.

>  	/* as per RFC 7112 section 5, the entire IPv6 Header Chain must fit
>  	 * the first fragment
>  	 */
> @@ -1635,10 +1659,11 @@ static int __ip6_append_data(struct sock *sk,
>  			else
>  				fraggap = 0;
>  			/* update mtu and maxfraglen if necessary */
> -			if (!skb || !skb_prev)
> -				ip6_append_data_mtu(&mtu, &maxfraglen,
> -						    fragheaderlen, skb, rt,
> -						    orig_mtu);
> +			if ((!skb || !skb_prev) &&
> +			    !ip6_append_data_mtu(&mtu, &maxfraglen,
> +						 fragheaderlen, skb, rt,
> +						 orig_mtu))
> +				goto emsgsize;
>  

The emsgsize label returns directly, which skips the cleanup done under "error:".
Could this, for example, leak uarg if msg_zerocopy_realloc() already allocated it?
If ip6_append_data_mtu() remains unchanged, you could remove this change
too.

>  			skb_prev = skb;
>