I2P Address: [http://git.idk.i2p]

Skip to content
Snippets Groups Projects
Commit 213bc4bb authored by zzz's avatar zzz
Browse files

- Prevent release of TunnelDataMessage cached ByteArray, 

        as it may be reused if retried in another transport;
        a nasty bug causing corrupt messages
      - Lots of code to detect improper reuse of cached objects
        after release
parent 1a01aa0a
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
router/java/src/net/i2p/data/i2np/TunnelDataMessage.java
+ 79
− 4
View file @ 213bc4bb
......@@ -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;
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment

Base32 Address: [http://7qeve4v2chmjdqlwpa3vl7aojf3nodbku7vepnjwrsxljzqipz6a.b32.i2p] Onion Address: [http://47ggr2fa3vnwfyhvgskzdmr3i32eijwymxohtxsls45dulmriwxszjad.onion]