[14] RFR JDK-8228835 "Memory leak in PKCS11 provider when using AES GCM"

[Jamil Nimeh]

Nope, no other comments, looks good to me.

--Jamil

On 8/13/2019 2:21 PM, Valerie Peng wrote:
>
> Hi Jamil,
>
> Thanks for the review, webrev updated at 
> http://cr.openjdk.java.net/~valeriep/8228835/webrev.01/
>
> More replies in line below.
>
> On 8/8/2019 5:30 PM, Jamil Nimeh wrote:
>>
>> Hi Valerie, looks pretty good.  I only had a few minor comments.
>>
>>   * pkcs11wrapper.h
>>       o 210-212: Do you intend to leave these trace macros in there
>>         but commented?  It seems like they could be safely left
>>         uncommented in case you needed to use them for debugging.
>>       o 388: This looks like a duplicate of line 387.
>>
> Yes, I have used those trace macros quite extensively, so I left them 
> in and commented out. More convenient this way.
>
> Removed line 388.
>
>>   * libj2pkcs11.c
>>       o 271/277 (and others) - If you know you're going to zero the
>>         memory right off the bat you could use calloc() rather than
>>         malloc and get the benefit of the memory being zeroed
>>         automatically.  Just a suggestion, there's nothing wrong with
>>         the code as it is.
>>       o 395: nit - extra space between ckpdate and ";"
>>       o 843: unnecessary double-parenthesis
>>
> Fixed.
>
> Mach5 run is clean. Please let me know if you have more comments.
>
> Thanks,
>
> Valerie
>
>> --Jamil
>>
>> On 8/6/19 7:28 PM, Valerie Peng wrote:
>>>
>>> Anyone have spare cycles for reviewing this?
>>>
>>> The current PKCS11 JNI code for handling native mechanism and its 
>>> parameters is a bit too all over the place. To make the tracing and 
>>> verification easier, I consolidated the memory deallocation to the 
>>> freeCKMechanismPtr(...) method in p11_util.c (some was in 
>>> p11_keymgmt.c).
>>>
>>> Also, fixed the j_XXX_ToCK_XXXX_ methods in p11_convert.c to 
>>> allocate the memory inside each method and return NULL upon error 
>>> and make sure allocated memories are free'd upon any failure.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8228835
>>>
>>> Webrev: http://cr.openjdk.java.net/~valeriep/8228835/webrev.00/
>>>
>>> Mach5 run is clean.
>>>
>>> Thanks,
>>> Valerie

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20190813/1c4165d2/attachment.htm>