diff --git a/router/java/src/net/i2p/data/i2np/TunnelDataMessage.java b/router/java/src/net/i2p/data/i2np/TunnelDataMessage.java
index a55efc2b1fd40292ed79e00c3e1b609125d8b85c..70876ea64c1aab5776990e4dea9866640e477216 100644
--- a/router/java/src/net/i2p/data/i2np/TunnelDataMessage.java
+++ b/router/java/src/net/i2p/data/i2np/TunnelDataMessage.java
@@ -33,7 +33,7 @@ public class TunnelDataMessage extends I2NPMessageImpl {
/** if we can't deliver a tunnel message in 10s, fuck it */
private static final int EXPIRATION_PERIOD = 10*1000;
- private static final ByteCache _cache = ByteCache.getInstance(512, DATA_SIZE);
+ private static final ByteCache _cache;
/**
* When true, it means this tunnelDataMessage is being used as part of a tunnel
* processing pipeline, where the byte array is acquired during the TunnelDataMessage's
@@ -42,9 +42,63 @@ public class TunnelDataMessage extends I2NPMessageImpl {
* handler's cache, etc), until it is finally released back into the cache when written
* to the next peer (or explicitly by the fragment handler's completion).
* Setting this to false just increases memory churn
+ *
+ * Well, this is tricky to get right and avoid data corruption,
+ * here's an example after checks were put in:
+ *
+ *
+ 10:57:05.197 CRIT [NTCP read 1 ] 2p.data.i2np.TunnelDataMessage: TDM boom
+ net.i2p.data.i2np.I2NPMessageException: TDM data buf use after free
+ at net.i2p.data.i2np.TunnelDataMessage.writeMessageBody(TunnelDataMessage.java:124)
+ at net.i2p.data.i2np.I2NPMessageImpl.toByteArray(I2NPMessageImpl.java:217)
+ at net.i2p.router.transport.ntcp.NTCPConnection.bufferedPrepare(NTCPConnection.java:678)
+ at net.i2p.router.transport.ntcp.NTCPConnection.send(NTCPConnection.java:293)
+ at net.i2p.router.transport.ntcp.NTCPTransport.outboundMessageReady(NTCPTransport.java:185)
+ at net.i2p.router.transport.TransportImpl.send(TransportImpl.java:357)
+ at net.i2p.router.transport.GetBidsJob.getBids(GetBidsJob.java:80)
+ at net.i2p.router.transport.CommSystemFacadeImpl.processMessage(CommSystemFacadeImpl.java:129)
+ at net.i2p.router.OutNetMessagePool.add(OutNetMessagePool.java:61)
+ at net.i2p.router.transport.TransportImpl.afterSend(TransportImpl.java:252)
+ at net.i2p.router.transport.TransportImpl.afterSend(TransportImpl.java:163)
+ at net.i2p.router.transport.udp.UDPTransport.failed(UDPTransport.java:1314)
+ at net.i2p.router.transport.udp.PeerState.add(PeerState.java:1064)
+ at net.i2p.router.transport.udp.OutboundMessageFragments.add(OutboundMessageFragments.java:146)
+ at net.i2p.router.transport.udp.UDPTransport.send(UDPTransport.java:1098)
+ at net.i2p.router.transport.GetBidsJob.getBids(GetBidsJob.java:80)
+ at net.i2p.router.transport.CommSystemFacadeImpl.processMessage(CommSystemFacadeImpl.java:129)
+ at net.i2p.router.OutNetMessagePool.add(OutNetMessagePool.java:61)
+ at net.i2p.router.tunnel.TunnelParticipant.send(TunnelParticipant.java:172)
+ at net.i2p.router.tunnel.TunnelParticipant.dispatch(TunnelParticipant.java:86)
+ at net.i2p.router.tunnel.TunnelDispatcher.dispatch(TunnelDispatcher.java:351)
+ at net.i2p.router.InNetMessagePool.doShortCircuitTunnelData(InNetMessagePool.java:306)
+ at net.i2p.router.InNetMessagePool.shortCircuitTunnelData(InNetMessagePool.java:291)
+ at net.i2p.router.InNetMessagePool.add(InNetMessagePool.java:160)
+ at net.i2p.router.transport.TransportManager.messageReceived(TransportManager.java:462)
+ at net.i2p.router.transport.TransportImpl.messageReceived(TransportImpl.java:416)
+ at net.i2p.router.transport.ntcp.NTCPConnection$ReadState.receiveLastBlock(NTCPConnection.java:1285)
+ at net.i2p.router.transport.ntcp.NTCPConnection$ReadState.receiveSubsequent(NTCPConnection.java:1248)
+ at net.i2p.router.transport.ntcp.NTCPConnection$ReadState.receiveBlock(NTCPConnection.java:1205)
+ at net.i2p.router.transport.ntcp.NTCPConnection.recvUnencryptedI2NP(NTCPConnection.java:1035)
+ at net.i2p.router.transport.ntcp.NTCPConnection.recvEncryptedI2NP(NTCPConnection.java:1018)
+ at net.i2p.router.transport.ntcp.Reader.processRead(Reader.java:167)
+ at net.i2p.router.transport.ntcp.Reader.access$400(Reader.java:17)
+ at net.i2p.router.transport.ntcp.Reader$Runner.run(Reader.java:106)
+ at java.lang.Thread.run(Thread.java:619)
+ at net.i2p.util.I2PThread.run(I2PThread.java:71)
+ *
*/
private static final boolean PIPELINED_CACHE = true;
+ static {
+ if (PIPELINED_CACHE)
+ _cache = ByteCache.getInstance(512, DATA_SIZE);
+ else
+ _cache = null;
+ }
+
+ /** For use-after-free checks. Always false if PIPELINED_CACHE is false. */
+ private boolean _hadCache;
+
public TunnelDataMessage(I2PAppContext context) {
super(context);
_log = context.logManager().getLog(TunnelDataMessage.class);
@@ -63,7 +117,15 @@ public class TunnelDataMessage extends I2NPMessageImpl {
_tunnelId = id.getTunnelId();
}
- public byte[] getData() { return _data; }
+ public byte[] getData() {
+ if (_hadCache && _dataBuf == null) {
+ RuntimeException e = new RuntimeException("TDM data buf use after free");
+ _log.error("TDM boom", e);
+ throw e;
+ }
+ return _data;
+ }
+
public void setData(byte data[]) {
if ( (data == null) || (data.length <= 0) )
throw new IllegalArgumentException("Empty tunnel payload?");
@@ -86,6 +148,7 @@ public class TunnelDataMessage extends I2NPMessageImpl {
if (PIPELINED_CACHE) {
_dataBuf = _cache.acquire();
_data = _dataBuf.getData();
+ _hadCache = true;
} else {
_data = new byte[DATA_SIZE];
}
@@ -101,12 +164,24 @@ public class TunnelDataMessage extends I2NPMessageImpl {
if (_data.length <= 0)
throw new I2NPMessageException("Not enough data to write out (data.length=" + _data.length + ")");
+ if (_hadCache && _dataBuf == null) {
+ I2NPMessageException e = new I2NPMessageException("TDM data buf use after free");
+ _log.error("TDM boom", e);
+ throw e;
+ }
+
DataHelper.toLong(out, curIndex, 4, _tunnelId);
curIndex += 4;
System.arraycopy(_data, 0, out, curIndex, DATA_SIZE);
curIndex += _data.length;
- if (PIPELINED_CACHE)
- _cache.release(_dataBuf);
+
+ // We can use from the cache, we just can't release to the cache, due to the bug
+ // noted above. In effect, this means that transmitted TDMs don't get their
+ // dataBufs released - but received TDMs do (via FragmentHandler)
+ //if (_hadCache) {
+ // _cache.release(_dataBuf);
+ // _dataBuf = null;
+ //}
return curIndex;
}