On 26.10.2020 13:55, Hangbin Liu wrote:
> On Mon, Oct 26, 2020 at 08:09:21AM +0000, Georg Kohmann (geokohma) wrote:
>>> + 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.
>
>>> +
>>> + 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
