On 14 Aug 2018, at 19:17, Jonathan T. Looney wrote:
Author: jtl
Date: Tue Aug 14 17:17:37 2018
New Revision: 337776
URL: https://svnweb.freebsd.org/changeset/base/337776

Log:
Improve IPv6 reassembly performance by hashing fragments into buckets.

  Currently, all IPv6 fragment reassembly queues are kept in a flat
  linked list. This has a number of implications. Two significant
  implications are: all reassembly operations share a common lock,
  and it is possible for the linked list to grow quite large.

Improve IPv6 reassembly performance by hashing fragments into buckets, each of which has its own lock. Calculate the hash key using a Jenkins
  hash with a random seed.

  Reviewed by:  jhb
  Security:     FreeBSD-SA-18:10.ip
  Security:     CVE-2018-6923

Modified:
  head/sys/netinet6/frag6.c

Modified: head/sys/netinet6/frag6.c
==============================================================================
--- head/sys/netinet6/frag6.c   Tue Aug 14 17:15:47 2018        (r337775)
+++ head/sys/netinet6/frag6.c   Tue Aug 14 17:17:37 2018        (r337776)
@@ -157,12 +176,13 @@ frag6_input(struct mbuf **mp, int *offp, int proto)
        struct mbuf *m = *mp, *t;
        struct ip6_hdr *ip6;
        struct ip6_frag *ip6f;
-       struct ip6q *q6;
+       struct ip6q *head, *q6;
        struct ip6asfrag *af6, *ip6af, *af6dwn;
        struct in6_ifaddr *ia;
        int offset = *offp, nxt, i, next;
        int first_frag = 0;
        int fragoff, frgpartlen;        /* must be larger than u_int16_t */
+       uint32_t hash, hashkey[sizeof(struct in6_addr) * 2 + 1], *hashkeyp;

I’m pretty sure you didn’t mean for the hashkey to be 1028 bytes long.

        struct ifnet *dstifp;
        u_int8_t ecn, ecn0;
 #ifdef RSS
@@ -231,7 +251,16 @@ frag6_input(struct mbuf **mp, int *offp, int proto)
                return (ip6f->ip6f_nxt);
        }

-       IP6Q_LOCK();
+       hashkeyp = hashkey;
+       memcpy(hashkeyp, &ip6->ip6_src, sizeof(struct in6_addr));
+       hashkeyp += sizeof(struct in6_addr) / sizeof(*hashkeyp);
+       memcpy(hashkeyp, &ip6->ip6_dst, sizeof(struct in6_addr));
+       hashkeyp += sizeof(struct in6_addr) / sizeof(*hashkeyp);
+       *hashkeyp = ip6f->ip6f_ident;
+       hash = jenkins_hash32(hashkey, nitems(hashkey), V_ip6q_hashseed);

Especially combined with this, because it means we hash a lot more than the src/dst dress and ident. We end up hashing random stack garbage, so the hash isn’t guaranteed to be the same every time.
So we end up not always being able to reassemble the packet.

It’s especially fun when you consider that a Dtrace fbt:::entry probe will leave a consistent stack state, so when you try to probe frag6_input() the problem magically goes away.

This broke the sys.netpfil.pf.fragmentation.v6 test.

I’ve done this, which fixes the problem:

        diff --git a/sys/netinet6/frag6.c b/sys/netinet6/frag6.c
        index 0f30801540a..e1f2b3f5842 100644
        --- a/sys/netinet6/frag6.c
        +++ b/sys/netinet6/frag6.c
        @@ -218,7 +218,9 @@ frag6_input(struct mbuf **mp, int *offp, int proto)
                int offset = *offp, nxt, i, next;
                int first_frag = 0;
int fragoff, frgpartlen; /* must be larger than u_int16_t */ - uint32_t hash, hashkey[sizeof(struct in6_addr) * 2 + 1], *hashkeyp; + uint32_t hashkey[(sizeof(struct in6_addr) * 2 + sizeof(u_int32_t)) /
        +           sizeof(uint32_t)];
        +       uint32_t hash, *hashkeyp;
                struct ifnet *dstifp;
                u_int8_t ecn, ecn0;
         #ifdef RSS

Regards,
Kristof
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to