From 8ed0dd2a5c4ff5c2ceb5a76297d9a75af292f311 Mon Sep 17 00:00:00 2001 From: zzz Date: Fri, 2 Aug 2019 15:48:05 +0000 Subject: [PATCH] Router: Fix Bloom filter false positives caused by lack of sync around buffers. Use temp buffers instead. Most likely to affect high-bandwidth or testnet routers; appears to be rare on lower-bandwidth routers. --- history.txt | 12 +++++ .../src/net/i2p/router/RouterVersion.java | 2 +- .../i2p/router/util/DecayingBloomFilter.java | 47 +++++++++---------- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/history.txt b/history.txt index de7e2278d..bcb9b10b4 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,15 @@ +2019-08-02 zzz + * Router: Fix Bloom filter false positives + +2019-08-01 zzz + * i2psnark: + - Fix announce hosts of the form b64dest[.i2p] + - Add last activity stat + - Disallow illegal filenames on Windows + +2019-07-27 zzz + * JBigI: GMP 6.1.2 for linux 64 bit (ticket #1869) + 2019-07-26 zzz * Debian: Change debian files from stretch (Jetty 9.2, Tomcat 8, Java 8) to buster (Jetty 9.4, Tomcat 9, Java 11) diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index f6e6df3a5..4e28a2c8d 100644 --- a/router/java/src/net/i2p/router/RouterVersion.java +++ b/router/java/src/net/i2p/router/RouterVersion.java @@ -18,7 +18,7 @@ public class RouterVersion { /** deprecated */ public final static String ID = "Monotone"; public final static String VERSION = CoreVersion.VERSION; - public final static long BUILD = 4; + public final static long BUILD = 5; /** for example "-test" */ public final static String EXTRA = ""; diff --git a/router/java/src/net/i2p/router/util/DecayingBloomFilter.java b/router/java/src/net/i2p/router/util/DecayingBloomFilter.java index 0afe7450d..c2f2bb50c 100644 --- a/router/java/src/net/i2p/router/util/DecayingBloomFilter.java +++ b/router/java/src/net/i2p/router/util/DecayingBloomFilter.java @@ -32,8 +32,6 @@ public class DecayingBloomFilter { protected final int _durationMs; protected final int _entryBytes; private final byte _extenders[][]; - private final byte _extended[]; - private final byte _longToEntry[]; private final long _longToEntryMask; protected long _currentDuplicates; protected volatile boolean _keepDecaying; @@ -57,8 +55,6 @@ public class DecayingBloomFilter { _durationMs = durationMs; // all final _extenders = null; - _extended = null; - _longToEntry = null; _longToEntryMask = 0; context.addShutdownTask(new Shutdown()); _keepDecaying = true; @@ -121,17 +117,15 @@ public class DecayingBloomFilter { int numExtenders = (32+ (entryBytes-1))/entryBytes - 1; if (numExtenders < 0) numExtenders = 0; - _extenders = new byte[numExtenders][entryBytes]; - for (int i = 0; i < numExtenders; i++) - _context.random().nextBytes(_extenders[i]); if (numExtenders > 0) { - _extended = new byte[32]; - _longToEntry = new byte[_entryBytes]; + _extenders = new byte[numExtenders][entryBytes]; + for (int i = 0; i < numExtenders; i++) { + _context.random().nextBytes(_extenders[i]); + } _longToEntryMask = (1l << (_entryBytes * 8l)) -1; } else { // final - _extended = null; - _longToEntry = null; + _extenders = null; _longToEntryMask = 0; } _keepDecaying = true; @@ -208,18 +202,19 @@ public class DecayingBloomFilter { */ public boolean add(long entry) { if (ALWAYS_MISS) return false; + byte[] longToEntry = new byte[_entryBytes]; if (_entryBytes <= 7) entry = ((entry ^ _longToEntryMask) & ((1 << 31)-1)) | (entry ^ _longToEntryMask); //entry &= _longToEntryMask; if (entry < 0) { - DataHelper.toLong(_longToEntry, 0, _entryBytes, 0-entry); - _longToEntry[0] |= (1 << 7); + DataHelper.toLong(longToEntry, 0, _entryBytes, 0-entry); + longToEntry[0] |= (1 << 7); } else { - DataHelper.toLong(_longToEntry, 0, _entryBytes, entry); + DataHelper.toLong(longToEntry, 0, _entryBytes, entry); } getReadLock(); try { - return locked_add(_longToEntry, 0, _longToEntry.length, true); + return locked_add(longToEntry, 0, _entryBytes, true); } finally { releaseReadLock(); } } @@ -230,28 +225,30 @@ public class DecayingBloomFilter { */ public boolean isKnown(long entry) { if (ALWAYS_MISS) return false; + byte[] longToEntry = new byte[_entryBytes]; if (_entryBytes <= 7) entry = ((entry ^ _longToEntryMask) & ((1 << 31)-1)) | (entry ^ _longToEntryMask); if (entry < 0) { - DataHelper.toLong(_longToEntry, 0, _entryBytes, 0-entry); - _longToEntry[0] |= (1 << 7); + DataHelper.toLong(longToEntry, 0, _entryBytes, 0-entry); + longToEntry[0] |= (1 << 7); } else { - DataHelper.toLong(_longToEntry, 0, _entryBytes, entry); + DataHelper.toLong(longToEntry, 0, _entryBytes, entry); } getReadLock(); try { - return locked_add(_longToEntry, 0, _longToEntry.length, false); + return locked_add(longToEntry, 0, _entryBytes, false); } finally { releaseReadLock(); } } private boolean locked_add(byte entry[], int offset, int len, boolean addIfNew) { - if (_extended != null) { + if (_extenders != null) { // extend the entry to 32 bytes - System.arraycopy(entry, offset, _extended, 0, len); - for (int i = 0; i < _extenders.length; i++) - DataHelper.xor(entry, offset, _extenders[i], 0, _extended, _entryBytes * (i+1), _entryBytes); - - BloomSHA1.FilterKey key = _current.getFilterKey(_extended, 0, 32); + byte[] extended = new byte[32]; + System.arraycopy(entry, offset, extended, 0, len); + for (int i = 0; i < _extenders.length; i++) { + DataHelper.xor(entry, offset, _extenders[i], 0, extended, _entryBytes * (i+1), _entryBytes); + } + BloomSHA1.FilterKey key = _current.getFilterKey(extended, 0, 32); boolean seen = _current.locked_member(key); if (!seen) seen = _previous.locked_member(key);