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

[Jamil Nimeh]

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.
  * 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

--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/20190808/8fd4e0ce/attachment.htm>