On 26.10.2020 13:55, Hangbin Liu wrote:

On Mon, Oct 26, 2020 at 08:09:21AM +0000, Georg Kohmann (geokohma) wrote:

quoted

quoted

+	nexthdr = hdr->nexthdr;
+	offset = ipv6_skip_exthdr(skb, skb_transport_offset(skb), &nexthdr, &frag_off);
+	if (offset < 0)
+		goto fail_hdr;
+
+	/* 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;

Maybe also check the special case IPPROTO_NONE?

IPPROTO_NONE defines the same with NEXTHDR_NONE. So ipv6_skip_exthdr() will
return -1, and we will goto fail_hdr and send ICMP parameter error message.

The question is if it's OK to reply a ICMP error for fragment + IPPROTO_NONE
packet? For pure IPPROTO_NONE message, we should drop silently, but what about
fragment message?

According to RFC8200 section 4.7: "If the Payload Length field of the IPv6

header indicates the presence of octets past the end of a header whose

Next Header field contains 59, those octets must be ignored and passed

on unchanged if the packet is forwarded." I have not found any RFC

describing different behaviour for fragmented packets.

quoted

quoted

+
+	if (frag_off == htons(IP6_MF) && 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) {

Are you planning to also add this fix for the fragmentation handling in the netfilter?

I have no plan to fix this on netfilter as netfilter is a module.
It may have different behavior during defragment.

Thanks
Hangbin

I might have a look at the netfilter myself then.


Thanks

Georg