On Thu, Oct 22, 2020 at 5:12 AM Hangbin Liu [off-list ref] wrote:

Hi Willem,

Thanks for the comments, see replies below.

On Wed, Oct 21, 2020 at 10:02:55AM -0400, Willem de Bruijn wrote:

quoted

quoted

+       is_frag = (ipv6_find_hdr(skb, &offs, NEXTHDR_FRAGMENT, NULL, NULL) == NEXTHDR_FRAGMENT);
+

ipv6_skip_exthdr already walks all headers. Should we not already see
frag_off != 0 if skipped over a fragment header? Analogous to the test
in ipv6_frag_rcv below.

Ah, yes, I forgot we can use this check.

quoted

quoted

+       nexthdr = hdr->nexthdr;
+       offset = ipv6_skip_exthdr(skb, skb_transport_offset(skb), &nexthdr, &frag_off);
+       if (offset >= 0 && frag_off == htons(IP6_MF) && (offset + 1) > skb->len) {

Offset +1 does not fully test "all headers through an upper layer
header". You note the caveat in your commit message. Perhaps for the
small list of common protocols at least use a length derived from
nexthdr?

Do you mean check the header like

if (nexthdr == IPPROTO_ICMPV6)
        offset = offset + seizeof(struct icmp6hdr);
else if (nexthdr == ...)
        offset = ...
else
        offset += 1;

if (frag_off == htons(IP6_MF) && offset > skb->len) {
        icmpv6_param_prob(skb, ICMPV6_HDR_INCOMP, 0);
        return -1;
}

Another questions is how to define the list, does TCP/UDP/SCTP/ICMPv6 enough?

Exactly. But only if it's possible without adding a ton of #include's.
It is best effort.

If feasible, TCP + UDP alone would suffice to cover most traffic.