[PATCH net v3] ipv6/netfilter: Discard first fragment not including all headers

[PATCH net v3] ipv6/netfilter: Discard first fragment not including all headers

[Georg Kohmann]

Packets are processed even though the first fragment don't include all
headers through the upper layer header. This breaks TAHI IPv6 Core
Conformance Test v6LC.1.3.6.

Referring to RFC8200 SECTION 4.5: "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."

The fragment needs to be validated the same way it is done in
commit 2efdaaaf883a ("IPv6: reply ICMP error if the first fragment don't
include all headers") for ipv6. Wrap the validation into a common function,
ipv6_frag_validate(). A closer inspection of the existing validation show
that it does not fullfill all aspects of RFC 8200, section 4.5, but is at
the moment sufficient to pass mentioned TAHI test.

In netfilter, utilize the fragment offset returned by find_prev_fhdr() to
let ipv6_frag_validate() start it's traverse from the fragment header.

Return 0 to drop the fragment in the netfilter. This is the same behaviour
as used on other protocol errors in this function, e.g. when
nf_ct_frag6_queue() returns -EPROTO. The Fragment will later be picked up
by ipv6_frag_rcv() in reassembly.c. ipv6_frag_rcv() will then send an
appropriate ICMP Parameter Problem message back to the source.

References commit 2efdaaaf883a ("IPv6: reply ICMP error if the first
fragment don't include all headers")
Signed-off-by: Georg Kohmann <geokohma@cisco.com>
---

Notes:
    v2: Wrap fragment validation code into exthdrs_code.c for use by both ipv6 and
    netfiter.
    
    v3: Remove unused variable frag_off from ipv6_frag_rcv().

 include/net/ipv6.h                      |  2 ++
 net/ipv6/exthdrs_core.c                 | 43 +++++++++++++++++++++++++++++++++
 net/ipv6/netfilter/nf_conntrack_reasm.c |  9 +++++++
 net/ipv6/reassembly.c                   | 26 +++++---------------
 4 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index bd1f396..489f3f9 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1064,6 +1064,8 @@ int ipv6_skip_exthdr(const struct sk_buff *, int start, u8 *nexthdrp,
 
 bool ipv6_ext_hdr(u8 nexthdr);
 
+bool ipv6_frag_validate(struct sk_buff *skb, int start, u8 *nexthdrp);
+
 enum {
 	IP6_FH_F_FRAG		= (1 << 0),
 	IP6_FH_F_AUTH		= (1 << 1),
diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
index da46c42..7a94fdf 100644
--- a/net/ipv6/exthdrs_core.c
+++ b/net/ipv6/exthdrs_core.c
@@ -278,3 +278,46 @@ int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
 	return nexthdr;
 }
 EXPORT_SYMBOL(ipv6_find_hdr);
+
+/* Validate that the upper layer header is not truncated in fragment.
+ *
+ * This function returns false if a TCP, UDP or ICMP header is truncated
+ * just before or in the middle of the header. It also returns false if
+ * any other upper layer header is truncated just before the first byte.
+ *
+ * Notes:
+ * -It does NOT return false if the first fragment where truncated
+ * elsewhere, i.e. between or in the middle of one of the extension
+ * headers or in the middle of one of the upper layer headers, except for
+ * TCP, UDP and ICMP.
+ * -The function also returns true if the fragment is not the first
+ * fragment.
+ */
+
+bool ipv6_frag_validate(struct sk_buff *skb, int start, u8 *nexthdrp)
+{
+	int offset;
+	u8 nexthdr = *nexthdrp;
+	__be16 frag_off;
+
+	offset = ipv6_skip_exthdr(skb, start, &nexthdr, &frag_off);
+	if (offset >= 0 && !(frag_off & htons(IP6_OFFSET))) {
+		switch (nexthdr) {
+		case NEXTHDR_TCP:
+			offset += sizeof(struct tcphdr);
+			break;
+		case NEXTHDR_UDP:
+			offset += sizeof(struct udphdr);
+			break;
+		case NEXTHDR_ICMP:
+			offset += sizeof(struct icmp6hdr);
+			break;
+		default:
+			offset += 1;
+		}
+		if (offset > skb->len)
+			return false;
+	}
+	return true;
+}
+EXPORT_SYMBOL(ipv6_frag_validate);
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 054d287..f6cae28 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -445,6 +445,7 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
 	struct frag_queue *fq;
 	struct ipv6hdr *hdr;
 	u8 prevhdr;
+	u8 nexthdr = NEXTHDR_FRAGMENT;
 
 	/* Jumbo payload inhibits frag. header */
 	if (ipv6_hdr(skb)->payload_len == 0) {
@@ -455,6 +456,14 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
 	if (find_prev_fhdr(skb, &prevhdr, &nhoff, &fhoff) < 0)
 		return 0;
 
+	/* Discard the first fragment if it does not include all headers
+	 * RFC 8200, Section 4.5
+	 */
+	if (!ipv6_frag_validate(skb, fhoff, &nexthdr)) {
+		pr_debug("Drop incomplete fragment\n");
+		return 0;
+	}
+
 	if (!pskb_may_pull(skb, fhoff + sizeof(*fhdr)))
 		return -ENOMEM;
 
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index c8cf1bb..04e078e 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -324,8 +324,7 @@ 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);
-	__be16 frag_off;
-	int iif, offset;
+	int iif;
 	u8 nexthdr;
 
 	if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
@@ -362,24 +361,11 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
 	 * 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;
-		}
+	if (!ipv6_frag_validate(skb, skb_transport_offset(skb), &nexthdr)) {
+		__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;
-- 
2.10.2

Re: [PATCH net v3] ipv6/netfilter: Discard first fragment not including all headers

[Jakub Kicinski]

On Mon,  9 Nov 2020 12:52:49 +0100 Georg Kohmann wrote:
> Packets are processed even though the first fragment don't include all
> headers through the upper layer header. This breaks TAHI IPv6 Core
> Conformance Test v6LC.1.3.6.
> 
> Referring to RFC8200 SECTION 4.5: "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."
> 
> The fragment needs to be validated the same way it is done in
> commit 2efdaaaf883a ("IPv6: reply ICMP error if the first fragment don't
> include all headers") for ipv6. Wrap the validation into a common function,
> ipv6_frag_validate(). A closer inspection of the existing validation show
> that it does not fullfill all aspects of RFC 8200, section 4.5, but is at
> the moment sufficient to pass mentioned TAHI test.
> 
> In netfilter, utilize the fragment offset returned by find_prev_fhdr() to
> let ipv6_frag_validate() start it's traverse from the fragment header.
> 
> Return 0 to drop the fragment in the netfilter. This is the same behaviour
> as used on other protocol errors in this function, e.g. when
> nf_ct_frag6_queue() returns -EPROTO. The Fragment will later be picked up
> by ipv6_frag_rcv() in reassembly.c. ipv6_frag_rcv() will then send an
> appropriate ICMP Parameter Problem message back to the source.
> 
> References commit 2efdaaaf883a ("IPv6: reply ICMP error if the first
> fragment don't include all headers")

new line here, since the line above is not really a tag.

> Signed-off-by: Georg Kohmann <geokohma@cisco.com>

> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index bd1f396..489f3f9 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -1064,6 +1064,8 @@ int ipv6_skip_exthdr(const struct sk_buff *, int start, u8 *nexthdrp,
>  
>  bool ipv6_ext_hdr(u8 nexthdr);
>  
> +bool ipv6_frag_validate(struct sk_buff *skb, int start, u8 *nexthdrp);
> +
>  enum {
>  	IP6_FH_F_FRAG		= (1 << 0),
>  	IP6_FH_F_AUTH		= (1 << 1),
> diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
> index da46c42..7a94fdf 100644
> --- a/net/ipv6/exthdrs_core.c
> +++ b/net/ipv6/exthdrs_core.c
> @@ -278,3 +278,46 @@ int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
>  	return nexthdr;
>  }
>  EXPORT_SYMBOL(ipv6_find_hdr);
> +
> +/* Validate that the upper layer header is not truncated in fragment.
> + *
> + * This function returns false if a TCP, UDP or ICMP header is truncated
> + * just before or in the middle of the header. It also returns false if
> + * any other upper layer header is truncated just before the first byte.
> + *
> + * Notes:
> + * -It does NOT return false if the first fragment where truncated

More spaces needed, i.e.

Notes:
 - It...

> + * elsewhere, i.e. between or in the middle of one of the extension
> + * headers or in the middle of one of the upper layer headers, except for
> + * TCP, UDP and ICMP.
> + * -The function also returns true if the fragment is not the first
> + * fragment.
> + */
> +

no need for a new line here

> +bool ipv6_frag_validate(struct sk_buff *skb, int start, u8 *nexthdrp)

(a) why place this function in exthdrs_core? I don't see any header
    specific code here, IMO it belongs in reassembly.c.

(b) the name is a bit broad, how about ipv6_frag_thdr_tuncated() or
    some such?

> +{
> +	int offset;
> +	u8 nexthdr = *nexthdrp;
> +	__be16 frag_off;

order these longest line to shortest (rev xmas tree) please.

> +
> +	offset = ipv6_skip_exthdr(skb, start, &nexthdr, &frag_off);
> +	if (offset >= 0 && !(frag_off & htons(IP6_OFFSET))) {

nit: since this is a function now you can reverse the condition, return
early, and save the indentation level in all the code below

> +		switch (nexthdr) {
> +		case NEXTHDR_TCP:
> +			offset += sizeof(struct tcphdr);
> +			break;
> +		case NEXTHDR_UDP:
> +			offset += sizeof(struct udphdr);
> +			break;
> +		case NEXTHDR_ICMP:
> +			offset += sizeof(struct icmp6hdr);
> +			break;
> +		default:
> +			offset += 1;
> +		}
> +		if (offset > skb->len)
> +			return false;
> +	}
> +	return true;
> +}
> +EXPORT_SYMBOL(ipv6_frag_validate);
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index 054d287..f6cae28 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -445,6 +445,7 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
>  	struct frag_queue *fq;
>  	struct ipv6hdr *hdr;
>  	u8 prevhdr;
> +	u8 nexthdr = NEXTHDR_FRAGMENT;

rev xmas tree

>  	/* Jumbo payload inhibits frag. header */
>  	if (ipv6_hdr(skb)->payload_len == 0) {
> @@ -455,6 +456,14 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
>  	if (find_prev_fhdr(skb, &prevhdr, &nhoff, &fhoff) < 0)
>  		return 0;
>  
> +	/* Discard the first fragment if it does not include all headers
> +	 * RFC 8200, Section 4.5
> +	 */
> +	if (!ipv6_frag_validate(skb, fhoff, &nexthdr)) {
> +		pr_debug("Drop incomplete fragment\n");
> +		return 0;
> +	}
>
>  	if (!pskb_may_pull(skb, fhoff + sizeof(*fhdr)))
>  		return -ENOMEM;
>  
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index c8cf1bb..04e078e 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -324,8 +324,7 @@ 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);
> -	__be16 frag_off;
> -	int iif, offset;
> +	int iif;

rev xmas tree

>  	u8 nexthdr;
>  
>  	if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
> @@ -362,24 +361,11 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>  	 * 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;
> -		}
> +	if (!ipv6_frag_validate(skb, skb_transport_offset(skb), &nexthdr)) {
> +		__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;

Re: [PATCH net v3] ipv6/netfilter: Discard first fragment not including all headers

[Georg Kohmann (geokohma)]

On 09.11.2020 21:50, Jakub Kicinski wrote:
> On Mon,  9 Nov 2020 12:52:49 +0100 Georg Kohmann wrote:
>> Packets are processed even though the first fragment don't include all
>> headers through the upper layer header. This breaks TAHI IPv6 Core
>> Conformance Test v6LC.1.3.6.
>>
>> Referring to RFC8200 SECTION 4.5: "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."
>>
>> The fragment needs to be validated the same way it is done in
>> commit 2efdaaaf883a ("IPv6: reply ICMP error if the first fragment don't
>> include all headers") for ipv6. Wrap the validation into a common function,
>> ipv6_frag_validate(). A closer inspection of the existing validation show
>> that it does not fullfill all aspects of RFC 8200, section 4.5, but is at
>> the moment sufficient to pass mentioned TAHI test.
>>
>> In netfilter, utilize the fragment offset returned by find_prev_fhdr() to
>> let ipv6_frag_validate() start it's traverse from the fragment header.
>>
>> Return 0 to drop the fragment in the netfilter. This is the same behaviour
>> as used on other protocol errors in this function, e.g. when
>> nf_ct_frag6_queue() returns -EPROTO. The Fragment will later be picked up
>> by ipv6_frag_rcv() in reassembly.c. ipv6_frag_rcv() will then send an
>> appropriate ICMP Parameter Problem message back to the source.
>>
>> References commit 2efdaaaf883a ("IPv6: reply ICMP error if the first
>> fragment don't include all headers")
> new line here, since the line above is not really a tag.
>
>> Signed-off-by: Georg Kohmann <geokohma@cisco.com>
>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
>> index bd1f396..489f3f9 100644
>> --- a/include/net/ipv6.h
>> +++ b/include/net/ipv6.h
>> @@ -1064,6 +1064,8 @@ int ipv6_skip_exthdr(const struct sk_buff *, int start, u8 *nexthdrp,
>>  
>>  bool ipv6_ext_hdr(u8 nexthdr);
>>  
>> +bool ipv6_frag_validate(struct sk_buff *skb, int start, u8 *nexthdrp);
>> +
>>  enum {
>>  	IP6_FH_F_FRAG		= (1 << 0),
>>  	IP6_FH_F_AUTH		= (1 << 1),
>> diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
>> index da46c42..7a94fdf 100644
>> --- a/net/ipv6/exthdrs_core.c
>> +++ b/net/ipv6/exthdrs_core.c
>> @@ -278,3 +278,46 @@ int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
>>  	return nexthdr;
>>  }
>>  EXPORT_SYMBOL(ipv6_find_hdr);
>> +
>> +/* Validate that the upper layer header is not truncated in fragment.
>> + *
>> + * This function returns false if a TCP, UDP or ICMP header is truncated
>> + * just before or in the middle of the header. It also returns false if
>> + * any other upper layer header is truncated just before the first byte.
>> + *
>> + * Notes:
>> + * -It does NOT return false if the first fragment where truncated
> More spaces needed, i.e.
>
> Notes:
>  - It...
>
>> + * elsewhere, i.e. between or in the middle of one of the extension
>> + * headers or in the middle of one of the upper layer headers, except for
>> + * TCP, UDP and ICMP.
>> + * -The function also returns true if the fragment is not the first
>> + * fragment.
>> + */
>> +
> no need for a new line here
>
>> +bool ipv6_frag_validate(struct sk_buff *skb, int start, u8 *nexthdrp)
> (a) why place this function in exthdrs_core? I don't see any header
>     specific code here, IMO it belongs in reassembly.c.

ipv6_frag_validate() is used in both reassembly.c and nf_conntrack_reasm.c
Where should I put the prototype so it can be used both places?

>
> (b) the name is a bit broad, how about ipv6_frag_thdr_tuncated() or
>     some such?
Yes agree, much better.
>> +{
>> +	int offset;
>> +	u8 nexthdr = *nexthdrp;
>> +	__be16 frag_off;
> order these longest line to shortest (rev xmas tree) please.
>
>> +
>> +	offset = ipv6_skip_exthdr(skb, start, &nexthdr, &frag_off);
>> +	if (offset >= 0 && !(frag_off & htons(IP6_OFFSET))) {
> nit: since this is a function now you can reverse the condition, return
> early, and save the indentation level in all the code below
>
>> +		switch (nexthdr) {
>> +		case NEXTHDR_TCP:
>> +			offset += sizeof(struct tcphdr);
>> +			break;
>> +		case NEXTHDR_UDP:
>> +			offset += sizeof(struct udphdr);
>> +			break;
>> +		case NEXTHDR_ICMP:
>> +			offset += sizeof(struct icmp6hdr);
>> +			break;
>> +		default:
>> +			offset += 1;
>> +		}
>> +		if (offset > skb->len)
>> +			return false;
>> +	}
>> +	return true;
>> +}
>> +EXPORT_SYMBOL(ipv6_frag_validate);
>> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
>> index 054d287..f6cae28 100644
>> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
>> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
>> @@ -445,6 +445,7 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
>>  	struct frag_queue *fq;
>>  	struct ipv6hdr *hdr;
>>  	u8 prevhdr;
>> +	u8 nexthdr = NEXTHDR_FRAGMENT;
> rev xmas tree
>
>>  	/* Jumbo payload inhibits frag. header */
>>  	if (ipv6_hdr(skb)->payload_len == 0) {
>> @@ -455,6 +456,14 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
>>  	if (find_prev_fhdr(skb, &prevhdr, &nhoff, &fhoff) < 0)
>>  		return 0;
>>  
>> +	/* Discard the first fragment if it does not include all headers
>> +	 * RFC 8200, Section 4.5
>> +	 */
>> +	if (!ipv6_frag_validate(skb, fhoff, &nexthdr)) {
>> +		pr_debug("Drop incomplete fragment\n");
>> +		return 0;
>> +	}
>>
>>  	if (!pskb_may_pull(skb, fhoff + sizeof(*fhdr)))
>>  		return -ENOMEM;
>>  
>> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
>> index c8cf1bb..04e078e 100644
>> --- a/net/ipv6/reassembly.c
>> +++ b/net/ipv6/reassembly.c
>> @@ -324,8 +324,7 @@ 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);
>> -	__be16 frag_off;
>> -	int iif, offset;
>> +	int iif;
> rev xmas tree
>
>>  	u8 nexthdr;
>>  
>>  	if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
>> @@ -362,24 +361,11 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>>  	 * 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;
>> -		}
>> +	if (!ipv6_frag_validate(skb, skb_transport_offset(skb), &nexthdr)) {
>> +		__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;
Thanks for reviewing, with good input. Please see question above.

Georg

Re: [PATCH net v3] ipv6/netfilter: Discard first fragment not including all headers

[Jakub Kicinski]

On Mon, 9 Nov 2020 22:08:47 +0000 Georg Kohmann (geokohma) wrote:
> >> +bool ipv6_frag_validate(struct sk_buff *skb, int start, u8 *nexthdrp)  
> > (a) why place this function in exthdrs_core? I don't see any header
> >     specific code here, IMO it belongs in reassembly.c.  
> 
> ipv6_frag_validate() is used in both reassembly.c and nf_conntrack_reasm.c
> Where should I put the prototype so it can be used both places?

The prototype can stay in net/ipv6.h