[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.