All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hangbin Liu <liuhangbin@gmail.com>
To: netdev@vger.kernel.org
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	Jakub Kicinski <kuba@kernel.org>,
	Willem de Bruijn <willemb@google.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Georg Kohmann <geokohma@cisco.com>,
	Hangbin Liu <liuhangbin@gmail.com>
Subject: [PATCHv6 net 2/2] IPv6: reply ICMP error if the first fragment don't include all headers
Date: Tue, 27 Oct 2020 20:33:13 +0800	[thread overview]
Message-ID: <20201027123313.3717941-3-liuhangbin@gmail.com> (raw)
In-Reply-To: <20201027123313.3717941-1-liuhangbin@gmail.com>

Based on RFC 8200, Section 4.5 Fragment Header:

  -  If the first fragment does not include all headers through an
     Upper-Layer header, then that fragment should be discarded and
     an ICMP Parameter Problem, Code 3, message should be sent to
     the source of the fragment, with the Pointer field set to zero.

Checking each packet header in IPv6 fast path will have performance impact,
so I put the checking in ipv6_frag_rcv().

As the packet may be any kind of L4 protocol, I only checked some common
protocols' header length and handle others by (offset + 1) > skb->len.
Also use !(frag_off & htons(IP6_OFFSET)) to catch atomic fragments
(fragmented packet with only one fragment).

When send ICMP error message, if the 1st truncated fragment is ICMP message,
icmp6_send() will break as is_ineligible() return true. So I added a check
in is_ineligible() to let fragment packet with nexthdr ICMP but no ICMP header
return false.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---

v6:
Use !(frag_off & htons(IP6_OFFSET)) to catch atomic fragments

v5:
Only check nexthdr if ipv6_skip_exthdr() does not return -1. For
IPPROTO_NONE/NEXTHDR_NONE, later code will handle and ignore it.

v4:
remove unused variable

v3:
a) use frag_off to check if this is a fragment packet
b) check some common protocols' header length

v2:
a) Move header check to ipv6_frag_rcv(). Also check the ipv6_skip_exthdr()
   return value
b) Fix ipv6_find_hdr() parameter type miss match in is_ineligible()

---
 net/ipv6/icmp.c       |  8 +++++++-
 net/ipv6/reassembly.c | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index ec448b71bf9a..8956144ea65e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -158,7 +158,13 @@ static bool is_ineligible(const struct sk_buff *skb)
 		tp = skb_header_pointer(skb,
 			ptr+offsetof(struct icmp6hdr, icmp6_type),
 			sizeof(_type), &_type);
-		if (!tp || !(*tp & ICMPV6_INFOMSG_MASK))
+
+		/* Based on RFC 8200, Section 4.5 Fragment Header, return
+		 * false if this is a fragment packet with no icmp header info.
+		 */
+		if (!tp && frag_off != 0)
+			return false;
+		else if (!tp || !(*tp & ICMPV6_INFOMSG_MASK))
 			return true;
 	}
 	return false;
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 1f5d4d196dcc..c8cf1bbad74a 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -42,6 +42,8 @@
 #include <linux/skbuff.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
 
 #include <net/sock.h>
 #include <net/snmp.h>
@@ -322,7 +324,9 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
 	struct frag_queue *fq;
 	const struct ipv6hdr *hdr = ipv6_hdr(skb);
 	struct net *net = dev_net(skb_dst(skb)->dev);
-	int iif;
+	__be16 frag_off;
+	int iif, offset;
+	u8 nexthdr;
 
 	if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
 		goto fail_hdr;
@@ -351,6 +355,33 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
 		return 1;
 	}
 
+	/* RFC 8200, Section 4.5 Fragment Header:
+	 * If the first fragment does not include all headers through an
+	 * Upper-Layer header, then that fragment should be discarded and
+	 * an ICMP Parameter Problem, Code 3, message should be sent to
+	 * the source of the fragment, with the Pointer field set to zero.
+	 */
+	nexthdr = hdr->nexthdr;
+	offset = ipv6_skip_exthdr(skb, skb_transport_offset(skb), &nexthdr, &frag_off);
+	if (offset >= 0) {
+		/* Check some common protocols' header */
+		if (nexthdr == IPPROTO_TCP)
+			offset += sizeof(struct tcphdr);
+		else if (nexthdr == IPPROTO_UDP)
+			offset += sizeof(struct udphdr);
+		else if (nexthdr == IPPROTO_ICMPV6)
+			offset += sizeof(struct icmp6hdr);
+		else
+			offset += 1;
+
+		if (!(frag_off & htons(IP6_OFFSET)) && offset > skb->len) {
+			__IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev),
+					IPSTATS_MIB_INHDRERRORS);
+			icmpv6_param_prob(skb, ICMPV6_HDR_INCOMP, 0);
+			return -1;
+		}
+	}
+
 	iif = skb->dev ? skb->dev->ifindex : 0;
 	fq = fq_find(net, fhdr->identification, hdr, iif);
 	if (fq) {
-- 
2.25.4


  parent reply	other threads:[~2020-10-27 12:33 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07  3:55 [PATCH net 0/2] IPv6: reply ICMP error with fragment doesn't contain all headers Hangbin Liu
2020-10-07  3:55 ` [PATCH net 1/2] ICMPv6: Add ICMPv6 Parameter Problem, code 3 definition Hangbin Liu
2020-10-07  3:55 ` [PATCH net 2/2] IPv6: reply ICMP error if the first fragment don't include all headers Hangbin Liu
2020-10-07  9:35   ` Eric Dumazet
2020-10-08  8:30     ` Hangbin Liu
2020-10-08  9:47       ` Eric Dumazet
2020-10-09 10:07         ` Hangbin Liu
2020-10-07 14:58   ` Jakub Kicinski
2020-10-08  8:36     ` Hangbin Liu
2020-10-21  4:20 ` [PATCHv2 net 0/2] IPv6: reply ICMP error with fragment doesn't contain " Hangbin Liu
2020-10-21  4:20   ` [PATCHv2 net 1/2] ICMPv6: Add ICMPv6 Parameter Problem, code 3 definition Hangbin Liu
2020-10-21  4:20   ` [PATCHv2 net 2/2] IPv6: reply ICMP error if the first fragment don't include all headers Hangbin Liu
2020-10-21 14:02     ` Willem de Bruijn
2020-10-22  9:12       ` Hangbin Liu
2020-10-22 15:46         ` Willem de Bruijn
2020-10-23  6:43   ` [PATCHv3 net 0/2] IPv6: reply ICMP error if fragment doesn't contain " Hangbin Liu
2020-10-23  6:43     ` [PATCHv3 net 1/2] ICMPv6: Add ICMPv6 Parameter Problem, code 3 definition Hangbin Liu
2020-10-23  6:43     ` [PATCHv3 net 2/2] IPv6: reply ICMP error if the first fragment doesn't include all headers Hangbin Liu
2020-10-23 18:18       ` Jakub Kicinski
2020-10-26  7:29     ` [PATCHv4 net 0/2] IPv6: reply ICMP error if fragment doesn't contain " Hangbin Liu
2020-10-26  7:29       ` [PATCHv4 net 1/2] ICMPv6: Add ICMPv6 Parameter Problem, code 3 definition Hangbin Liu
2020-10-26  7:29       ` [PATCHv4 net 2/2] IPv6: reply ICMP error if the first fragment don't include all headers Hangbin Liu
2020-10-26  8:09         ` Georg Kohmann (geokohma)
2020-10-26 12:55           ` Hangbin Liu
2020-10-26 14:49             ` Georg Kohmann (geokohma)
2020-10-27  2:28       ` [PATCHv5 net 0/2] IPv6: reply ICMP error if fragment doesn't contain " Hangbin Liu
2020-10-27  2:28         ` [PATCHv5 net 1/2] ICMPv6: Add ICMPv6 Parameter Problem, code 3 definition Hangbin Liu
2020-10-27  2:28         ` [PATCHv5 net 2/2] IPv6: reply ICMP error if the first fragment don't include all headers Hangbin Liu
2020-10-27  7:57           ` Georg Kohmann (geokohma)
2020-10-27  9:57             ` Hangbin Liu
2020-10-27 10:20               ` Georg Kohmann (geokohma)
2020-10-30 15:31               ` Willem de Bruijn
2020-10-30 18:39                 ` Georg Kohmann (geokohma)
2020-10-27 12:33         ` [PATCHv6 net 0/2] IPv6: reply ICMP error if fragment doesn't contain " Hangbin Liu
2020-10-27 12:33           ` [PATCHv6 net 1/2] ICMPv6: Add ICMPv6 Parameter Problem, code 3 definition Hangbin Liu
2020-10-27 12:33           ` Hangbin Liu [this message]
2020-10-31 21:12           ` [PATCHv6 net 0/2] IPv6: reply ICMP error if fragment doesn't contain all headers Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201027123313.3717941-3-liuhangbin@gmail.com \
    --to=liuhangbin@gmail.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=geokohma@cisco.com \
    --cc=kuba@kernel.org \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=netdev@vger.kernel.org \
    --cc=willemb@google.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.