On 30 Aug 2018, at 22:00, Kristof Provost wrote:
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)) /
Can we actually make it the size of the field rather than uint32_t (not
u_int32_t)? I guess not easily but at least change the type spelling
and leave a comment what it is?
+ 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"
