Alright, I will update the synopsis. I am not exactly sure if it's GCM
only as CipherFinal() call is used for all Ucrypto encryption/decryption.
The testcase is using AES/GCM, but I am not sure if the underlying
native code which has the problem is GCM specific.
The using outLen's reference as the bufOut is from Solaris team. Since
null output buffer is always used with 0 output length when java code
calls the JNI code, it should be ok. However, let me see where I can add
more checks.
Thanks,
Valerie
On 9/3/2015 7:20 AM, Anthony Scarpino wrote:
On Sep 2, 2015, at 10:50 PM, Anthony Scarpino<anthony.scarp...@oracle.com>
wrote:
On Sep 2, 2015, at 3:45 PM, Valerie Peng<valerie.p...@oracle.com> wrote:
Can someone help review this java workaround for Solaris memory leak bug in
Ucrypto library?
Essentially, the memory leak occurs when a null output buffer is specified when
doing encryption/decryption.
So, the workaround in OracleUcrypto provider is to use non-null output buffers.
Webrev: http://cr.openjdk.java.net/~valeriep/8130875/webrev.00/
The fix is verified by running a program for a while and observe the memory
usage.
Valerie
Not related to the code, I think the bug synopsis should be more specific to the
issue. It looks too eye-catching by saying java runs out of memory with that cipher
suite, when it’s the OS library not cleaning up correctly for a particular provider
when using AES GCM only. I would not be surprised if a future issue got incorrectly
linked because the synopsis was too generic. Maybe something like “OracleUcrypto
workaround for AES GCM with a null bufOut pointer during doFinal()"
As for the code, I’m a bit unsure about using outLen’s reference as the bufOut
pointer. However, after seeing there are checks to make sure outLen is zero
and it’s documented well that this is a workaround, I’m ok with this.
Tony
Thanks to Jamil for asking me about the webrev privately, I discounted a
concern because I misread the webrev.
So the comments are purely from the JNI code checks. Maybe there are checks in
the java code that prevent the below situations, but it’s still uneasy having
no protection in the JNI code
The change around 438, does not make sure outLen is zero, so if bufOut points
to the reference of outLen, it could start overwriting data or even somewhere
else with the offset also being unchecked. The check should fail if outLen is
not zero. Before the change, having bufOut being null we could depend on the
OS library to check, now giving it a false pointer, we should do more checking.
Tony
