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

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

[bo cao]

ipv6: fix dead half of the MTU guard in __ip6_append_data()

__ip6_append_data() validates the path MTU against fragheaderlen
before the fragmentation loop, but the second half of the guard is
always false: it compares (mtu - fragheaderlen) + fragheaderlen
against sizeof(struct frag_hdr) (8), while fragheaderlen is at least
1280 for any packet carrying a Hop-by-Hop options header of 1240
bytes.  An IPV6_MTU socket option that makes mtu - fragheaderlen
underflow below fragheaderlen therefore passes the guard, defeats the
datalen clamp (which wraps), and leads to skb_over_panic() from
skb_put().

The bounds are also recomputed in ip6_append_data_mtu() for the first
fragment: with a transport-mode xfrm (DST_XFRM_TUNNEL clear) the
ESP/AH header is counted in dst.header_len but not in fragheaderlen,
so the recompute can lower mtu below fragheaderlen again after the
entry guard has passed.  Nothing checks a lower bound after the
recompute.

Rework the guard to require 8 bytes of payload after the fragment
header in every non-final fragment -- maxfraglen >= fragheaderlen +
sizeof(struct frag_hdr) + 8 -- both at cork setup and after every
recompute in ip6_append_data_mtu(), which now reports failure instead
of silently producing bogus values.  This matches ip6_fragment()'s
existing "mtu < hlen + sizeof(struct frag_hdr) + 8" check and covers
the plain IPV6_MTU case as well as the transport-mode xfrm case.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: bo cao <treebacker@foxmail.com>
---
Changes since v1:
- also check the recomputed bounds in ip6_append_data_mtu(): with a
  transport-mode xfrm, ip6_append_data_mtu() lowers mtu via
  dst.header_len (ESP/AH is in dst.header_len but not in
  fragheaderlen) after the entry guard has passed, re-opening the
  same skb_over_panic() (review, High)
- require 8 bytes of payload after the fragment header in every
  non-final fragment (maxfraglen >= fragheaderlen +
  sizeof(struct frag_hdr) + 8), matching ip6_fragment(); the v1
  comparison accepted mtu - fragheaderlen in [9, 15], which leaves
  zero payload per non-final fragment (review, Medium)
- match the From: and Signed-off-by: names (review, Low)

Link: https://lore.kernel.org/netdev/179321375161.3134.12690504630171635307@kernel.org/

 net/ipv6/ip6_output.c | 41 ++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

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
@@ -1347,7 +1347,7 @@ static inline struct ipv6_rt_hdr *ip6_rthdr_dup(struct ipv6_rt_hdr *src,
 	return src ? kmemdup(src, (src->hdrlen + 1) * 8, gfp) : NULL;
 }
 
-static void ip6_append_data_mtu(unsigned int *mtu,
+static bool ip6_append_data_mtu(unsigned int *mtu,
 				int *maxfraglen,
 				unsigned int fragheaderlen,
 				struct sk_buff *skb,
@@ -1357,8 +1357,20 @@ static void ip6_append_data_mtu(unsigned int *mtu,
 	if (!(rt->dst.flags & DST_XFRM_TUNNEL)) {
 		if (!skb) {
 			/* first fragment, reserve header_len */
-			*mtu = orig_mtu - rt->dst.header_len;
+			int new_mtu = (int)orig_mtu - (int)rt->dst.header_len;
 
+			/*
+			 * With a transport-mode xfrm the ESP/AH header is
+			 * counted in dst.header_len but not in
+			 * fragheaderlen, so this recompute can push
+			 * maxfraglen below fragheaderlen (or wrap it),
+			 * defeating the datalen clamp and leading to
+			 * skb_over_panic() from skb_put().
+			 */
+			if (new_mtu < (int)(fragheaderlen +
+					    sizeof(struct frag_hdr) + 8))
+				return false;
+			*mtu = new_mtu;
 		} else {
 			/*
 			 * this fragment is not first, the headers
@@ -1369,6 +1381,8 @@ static void ip6_append_data_mtu(unsigned int *mtu,
 		*maxfraglen = ((*mtu - fragheaderlen) & ~7)
 			      + fragheaderlen - sizeof(struct frag_hdr);
 	}
+
+	return true;
 }
 
 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;
 
 	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;
+
 	/* 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;
 
 			skb_prev = skb;
 
-- 
2.39.5 (Apple Git-154)

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[bo cao]

Hi!

Both points, for the record:

How it was found: LLM-assisted manual review of the MTU guard in
__ip6_append_data().  The second half of the guard,
((mtu - fragheaderlen) & ~7) + fragheaderlen <=
sizeof(struct frag_hdr), is trivially false for any real
fragheaderlen (it compares an expression dominated by fragheaderlen
against the constant 8), which led to a closer look at what that
comparison was supposed to protect against.

It was actually triggered.  Reproduced on a stock Ubuntu
7.3.0-9-generic kernel (arm64, QEMU) with a plain raw socket
(CAP_NET_RAW), no KASAN needed:

  - setsockopt(IPV6_MTU, 1281) on a route whose fragheaderlen is 1280
    (a 1240-byte Hop-by-Hop options cmsg), MSG_MORE, egress device
    without NETIF_F_SG:

    skbuff: skb_over_panic: text:... len:61280 put:61280
            head:... tail:0xef78 end:0x6c0 dev:<NULL>
    ------------[ cut here ]------------
    kernel BUG at net/core/skbuff.c:214!
    Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
    CPU: 3 UID: 0 PID: 1378 Comm: ipv6_maxfraglen
    pc : skb_panic+0x5c/0x60
    Call trace:
     skb_panic+0x5c/0x60 (P)
     skb_put+0x9c/0xa8
     __ip6_append_data+0xf80/0x1948
     ip6_append_data+0x94/0x2a8
     rawv6_sendmsg+0x588/0x988
     inet_sendmsg+0x4c/0xa8
     __sock_sendmsg+0x78/0x100
     ____sys_sendmsg+0x268/0x308
     ...

    (put:61280 = 60000 payload + 1280 fragheaderlen, exactly the wrap
    predicted by the dead comparison)

  - negative control, same setup with IPV6_MTU = 1288 (delta 8, just
    outside the broken window): sendmsg() returns -EMSGSIZE cleanly,
    no crash.  The crash/-EMSGSIZE boundary sits exactly where the
    dead comparison should have been checking.

The v2 guard rejects both: maxfraglen < fragheaderlen +
sizeof(struct frag_hdr) + 8 -> -EMSGSIZE.  The xfrm transport-mode
variant pointed out in the review of v1 is code-inspection only; it
was not triggered at runtime.

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

[Narcisa Vasile]

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;
>  

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch adds a failure exit inside the fragmentation loop of
  __ip6_append_data() (net/ipv6/ip6_output.c:1662-1666).
- [Medium] The new entry guard in __ip6_append_data() counts the fragment
  header twice, and it disagrees with the patch's other new check in…
- [Medium] Both new checks reject a datagram whether or not it actually
  needs fragmenting.
- [Low] The commit message has several defects: - The subject line is
  repeated as the first line of the body.

--

ipv6: fix dead half of the MTU guard in __ip6_append_data()

This patch replaces the always-false second half of the MTU guard in
__ip6_append_data() with a check that requires payload room after the
fragment header. ip6_append_data_mtu() now returns false when the
first-fragment recompute (which subtracts dst.header_len) falls below a
similar bound. When that happens, the fragmentation loop jumps to the
emsgsize label.

Link: https://lore.kernel.org/netdev/179321375161.3134.12690504630171635307@kernel.org/

> ipv6: fix dead half of the MTU guard in __ip6_append_data()
>
> ipv6: fix dead half of the MTU guard in __ip6_append_data()

[Severity: Low]
This isn't a bug, but the subject line is repeated as the first line of
the body.

> __ip6_append_data() validates the path MTU against fragheaderlen
> before the fragmentation loop, but the second half of the guard is
> always false: it compares (mtu - fragheaderlen) + fragheaderlen
> against sizeof(struct frag_hdr) (8), while fragheaderlen is at least
> 1280 for any packet carrying a Hop-by-Hop options header of 1240
> bytes.

The old second half is false for every packet once mtu > fragheaderlen.
fragheaderlen is always at least sizeof(struct ipv6hdr) (40), which is
larger than sizeof(struct frag_hdr) (8).

Could the explanation say that directly, instead of citing a 1240 byte
Hop-by-Hop header? The specific example suggests the guard might be live
for other header sizes.

> An IPV6_MTU socket option that makes mtu - fragheaderlen
> underflow below fragheaderlen therefore passes the guard

This phrase reads as garbled. Isn't it maxfraglen that drops below
fragheaderlen or wraps?

Also, the Link: tag sits below the --- separator with the v1 changelog,
so git am would drop it.

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 25fe0ba98b1a6..ab4eee51f1737 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c

[ ... ]

> @@ -1357,8 +1357,20 @@ static void ip6_append_data_mtu(unsigned int *mtu,
>  	if (!(rt->dst.flags & DST_XFRM_TUNNEL)) {
>  		if (!skb) {
>  			/* first fragment, reserve header_len */
> -			*mtu = orig_mtu - rt->dst.header_len;
> +			int new_mtu = (int)orig_mtu - (int)rt->dst.header_len;
>
> +			/*

[ ... ]

> +			 */
> +			if (new_mtu < (int)(fragheaderlen +
> +					    sizeof(struct frag_hdr) + 8))
> +				return false;
> +			*mtu = new_mtu;

[ ... ]

> @@ -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;
>
>  	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;

[Severity: Medium]
Is the fragment header being counted twice here? maxfraglen already has
sizeof(struct frag_hdr) subtracted:

    maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen - 8

So maxfraglen < fragheaderlen + 8 + 8 is the same as
((mtu - fragheaderlen) & ~7) < 24. That requires 16 bytes of payload per
non-final fragment, not the 8 that the comment describes.

ip6_fragment() only requires mtu - hlen >= 16:

    if (mtu < hlen + sizeof(struct frag_hdr) + 8)
        goto fail_toobig;

The new check in ip6_append_data_mtu() compares mtu rather than
maxfraglen, so it also only requires 16. It accepts
new_mtu == fragheaderlen + 16 and then sets
maxfraglen = fragheaderlen + 8, a value this entry guard rejects.

That doesn't seem to match the commit message's statement that the same
bound applies "both at cork setup and after every recompute in
ip6_append_data_mtu()".

As a result, sends with mtu - fragheaderlen in [16, 23] now fail with
-EMSGSIZE. Before this patch they produced valid fragments with 8 bytes
of payload. One example is a fragheaderlen of 1264 with an MTU of 1280,
which an unprivileged user can reach with a large IPV6_RTHDR SRH.

Could both sites use the same formula? For example, compare mtu against
fragheaderlen + sizeof(struct frag_hdr) + 8, or compare maxfraglen
against fragheaderlen + 8.

[Severity: Medium]
Does this also reject datagrams that don't need fragmenting at all? The
check runs before anything is known about the datagram length, so any
send with mtu - fragheaderlen < 24 gets -EMSGSIZE.

For example, take mtu - fragheaderlen = 20, opt_flen = 0, and a UDP send
with 4 bytes of data (12 bytes including the UDP header). The old code
computed maxfraglen = fragheaderlen + 8. cork->length (12) <= mtu, so
the datalen clamp was skipped. The datagram went out as one unfragmented
packet of fragheaderlen + 12 bytes.

With this patch that send fails with -EMSGSIZE.

The new check in ip6_append_data_mtu() behaves the same way. On a
transport-mode xfrm route it rejects the first skb whenever
orig_mtu - dst.header_len - fragheaderlen < 16, even if the whole
datagram fits.

The skb_over_panic() only happens on the fragmenting path, when this
clamp is applied:

    if (datalen > (cork->length <= mtu ? mtu : maxfraglen) - fragheaderlen)
        datalen = maxfraglen - fragheaderlen - rt->dst.trailer_len;

ip6_fragment() likewise only applies its bound when it actually
fragments.

Should these checks only apply when the datagram needs fragmenting? If
not, could the commit message mention that single-packet sends in this
range are now refused?

> +
>  	/* 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;

[Severity: High]
Can this leak the zerocopy uarg? ip6_append_data_mtu() only returns
false in the !skb branch. That means this exit is reached through
"if (!skb) goto alloc_new_skb", after __ip6_append_data() has already
done its setup:

    } else if (sock_flag(sk, SOCK_ZEROCOPY)) {
        uarg = msg_zerocopy_realloc(sk, length, skb_zcopy(skb),
                                    false);
        ...
        extra_uref = !skb_zcopy(skb);
        ...
            skb_zcopy_set(skb, uarg, &extra_uref);
    ...
        tskey = atomic_inc_return(&sk->sk_tskey) - 1;
        hold_tskey = true;
    ...
    cork->length += length;
    if (!skb)
        goto alloc_new_skb;

The emsgsize label only reports the error and returns:

    emsgsize:
        pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
        ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
        return -EMSGSIZE;

The cleanup lives under the error label:

    error:
        net_zcopy_put_abort(uarg, extra_uref);
        cork->length -= length;
        ...
        if (hold_tskey)
            atomic_dec(&sk->sk_tskey);

With skb == NULL, skb_zcopy_set() does nothing, so no skb owns the new
uarg from msg_zerocopy_alloc(). Its sock_hold(sk), its optmem charge and
its locked_vm accounting would never be released. sk_zckey and sk_tskey
would be left with gaps, and cork->length would stay inflated.

One way to trigger this is a UDPv6 MSG_ZEROCOPY send on a route covered
by a transport-mode SA. The non-fragmentable headers have to be sized so
that orig_mtu - dst.header_len < fragheaderlen + 16, for example with an
IPV6_RTHDR SRH, which needs no capability. That is the same case this
hunk is meant to handle.

The pmtu reported here also looks off. *mtu is only written after the
new check passes, so the value given to ipv6_local_error() is computed
from orig_mtu and ignores dst.header_len. A sender that follows that
hint would fail again.

Would it work to set err = -EMSGSIZE, call ipv6_local_error() and then
goto error here? Alternatively, the first-fragment header_len check
could run before the uarg and tskey setup, since all of its inputs are
known at that point.

>
>  			skb_prev = skb;
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/tencent_D1F9D5069513D7D12F0FC2E55A32A9ED4C08%40qq.com