Skip to content

Commit 45e0016

Browse files
behrooz-stripecopybara-github
authored andcommitted
Merge #52 by behrooz-stripe
Fix memory leak in HkdfStreamingPrf Repetitive calls to `prf.compute()` cause a memory leak. For example the following toy app will drain memory quickly: ``` public class Main { public static void main(String[] args) throws GeneralSecurityException, IOException { System.out.println("Starting..." + ProcessHandle.current().pid()); String secretRawString = "...a sample HkdfPrfKey..." PrfConfig.register(); KeysetHandle tinkHandle = CleartextKeysetHandle.read(JsonKeysetReader.withString(secretRawString)); for (Long i = 0L; i < 1_000_000_000L; i++) { PrfSet prf = tinkHandle.getPrimitive(PrfSet.class); prf.computePrimary(i.toString().getBytes(), 32); } System.out.println("Done. Press any key to exit..."); System.in.read(); } } ``` this is because in `HkdfInputStream` class, the [allocated ByteBuffer](https://github.com/tink-crypto/tink-java/blob/1240f412cce662317f7fe9130a6c3a0e1fa6916a/src/main/java/com/google/crypto/tink/subtle/prf/HkdfStreamingPrf.java#L115) is a `direct` buffer vs. a `heap buffer`. Direct buffers are allocated _"by calling through to native OS allocs, bypassing the standard JVM heap... memory-storage areas of direct buffers are not subject to garbage collection because they are outside the standard JVM heap."_ See [here for more info](https://stackoverflow.com/questions/5670862/bytebuffer-allocate-vs-bytebuffer-allocatedirect). Now the question might be the capacity is set to zero (`buffer = ByteBuffer.allocateDirect(0);`) however the overhead involved the pointer itself adds up on MANY calls. This basically means repetitive calls to `prf.compute` are same as this toy app: ``` public class Main { public static void main(String[] args) { System.out.println("Starting..." + ProcessHandle.current().pid()); for (Long i = 0L; i < 1_000_000_000L; i++) { ByteBuffer byteBuffer = ByteBuffer.allocate(0); byteBuffer.mark(); } System.out.println("Done. Press any key to exit..."); } } ``` Which sure enough will drain memory in matter of seconds. In practice since prf compute takes more time and is not called in such aggressive for loops; the leak is a lot slower and harder to notice. In our application, we had many repetitive calls resulting in an eventual drainage of machine memory. I have run these apps under a memory profiler and have confirmed the same findings. Stacktrace: ``` at jdk.internal.misc.Unsafe.allocateMemory0([email protected]/Native Method) at jdk.internal.misc.Unsafe.allocateMemory([email protected]/Unsafe.java:630) at java.nio.DirectByteBuffer.<init>([email protected]/DirectByteBuffer.java:125) at java.nio.ByteBuffer.allocateDirect([email protected]/ByteBuffer.java:332) at com.google.crypto.tink.subtle.prf.HkdfStreamingPrf$HkdfInputStream.initialize(HkdfStreamingPrf.java:88) at com.google.crypto.tink.subtle.prf.HkdfStreamingPrf$HkdfInputStream.read(HkdfStreamingPrf.java:129) at com.google.crypto.tink.subtle.prf.PrfImpl.readBytesFromStream(PrfImpl.java:45) at com.google.crypto.tink.subtle.prf.PrfImpl.compute(PrfImpl.java:67) at com.google.crypto.tink.prf.PrfSet.computePrimary(PrfSet.java:44) ... ``` The fix is simply to swap `allocateDirect` with `allocate` which is heap managed and garbage collected. I've verified the fix does indeed solve the leak. Closes ##52 PiperOrigin-RevId: 734509742 Change-Id: Id149e7dd44c941cb6c04e7cba80ff6e0234639f5
1 parent c8b1aad commit 45e0016

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

src/main/java/com/google/crypto/tink/subtle/prf/HkdfStreamingPrf.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ private void initialize() throws GeneralSecurityException, IOException {
112112
}
113113
mac.update(ikm);
114114
prk = mac.doFinal();
115-
buffer = ByteBuffer.allocateDirect(0);
115+
buffer = ByteBuffer.allocate(0);
116116
buffer.mark();
117117
ctr = 0;
118118
}

0 commit comments

Comments
 (0)