[RFR]: JDK-8203182 - Release session if initialization of SunPKCS11 Signature fails

[Valerie Peng]

Hi Martin,

Thanks for covering additional scenarios.

The updated webrev looks good to me. I will sponsor your patch.

Thanks,
Valerie

On 5/24/2018 12:13 PM, Martin Balao wrote:
> Hi Valerie,
>
> Thanks for your review.
>
> These are the cases I identified:
>
>  * engineSign and engineVerify include a try-finally block that 
> releases the session under any circumstances.
>
>  * engineUpdate may leak. Session release added if failure occurs.
>
>  * cancelOperation may leak. If session has no objects, it's killed. 
> If it has, a "cancel operation by finishing it" code is executed. This 
> code can fail on sign/verify operations, and a session may be leaked. 
> It's not clear to me why this "cancel operation by finishing it" code 
> is needed, though. Added a try-finally block anyways.
>
> Webrev 01:
>
>  * 
> http://cr.openjdk.java.net/~mbalao/webrevs/8203182/8203182.webrev.01/ 
> <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.01/>
>  * 
> http://cr.openjdk.java.net/~mbalao/webrevs/8203182/8203182.webrev.01.zip 
> <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.01.zip>
>
> Look forward to your answer.
>
> Kind regards,
> Martin.-
>
> On Tue, May 15, 2018 at 7:30 PM, Valerie Peng <valerie.peng at oracle.com 
> <mailto:valerie.peng at oracle.com>> wrote:
>
>     Hi Martin,
>
>     Your fix only addresses the initialization case. Have you
>     considered fixing the same problem under difference calls?
>
>     Regards,
>     Valerie
>
>     On 5/14/2018 3:25 PM, Martin Balao wrote:
>
>         Hi,
>
>         Can I have a review for "JDK-8203182 - Release session if
>         initialization of SunPKCS11 Signature fails" [1]?
>
>          *
>         http://cr.openjdk.java.net/~mbalao/webrevs/8203182/8203182.webrev.00/
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00/>
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00/
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00/>>
>          *
>         http://cr.openjdk.java.net/~mbalao/webrevs/8203182/8203182.webrev.00.zip
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00.zip>
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00.zip
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00.zip>>
>
>         Thanks in advanced!
>
>         Martin.-
>
>         --
>         [1] - https://bugs.openjdk.java.net/browse/JDK-8203182
>         <https://bugs.openjdk.java.net/browse/JDK-8203182>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180531/3fd9456c/attachment.htm>