[9] RFR: 8130875 Out of memory when using TLS_RSA_WITH_AES_128_GCM_SHA256

[Valerie Peng]

For all the JavaCritical calls, it's impossible to have the combination 
of (bufOut==NULL && outLen!=0) as the outLen value is generated by VM 
based on bufOut. Anyway, just to play it safe, I added a line to set 
outLen to 0 just to match what is done for line 657.

http://cr.openjdk.java.net/~valeriep/8130875/webrev.01/

Thanks,
Valerie

On 9/3/2015 7:20 AM, Anthony Scarpino wrote:
>> On Sep 2, 2015, at 10:50 PM, Anthony Scarpino<anthony.scarpino at oracle.com>  wrote:
>>
>>
>>> On Sep 2, 2015, at 3:45 PM, Valerie Peng<valerie.peng at 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
>