[7u-dev] RFA for JDK-8204344: Release session if initialization of SunPKCS11 Signature fails

[Andrew Hughes]

On 5 June 2018 at 17:10, Martin Balao <mbalao at redhat.com> wrote:
> Hi,
>
> Here there is a backport of JDK-8203182 [1] to JDK7:
>
>  *
> http://cr.openjdk.java.net/~mbalao/webrevs/8203182/backports/7/8203182.webrev.01/
>  *
> http://cr.openjdk.java.net/~mbalao/webrevs/8203182/backports/7/8203182.webrev.01.zip
>
> Backport ticket: JDK-8204344 [2].
> JDK commit: http://hg.openjdk.java.net/jdk/jdk/rev/00ebc17f3cc6
> Review approval:
> http://mail.openjdk.java.net/pipermail/security-dev/2018-May/017224.html
>
> I'd be grateful if someone can approve it.
>
> Kind regards,
> Martin.-
>
> --
> [1] - https://bugs.openjdk.java.net/browse/JDK-8203182
> [2] - https://bugs.openjdk.java.net/browse/JDK-8204344

Looks good to me. It's a pretty simple fix, made a little clearer if
you do a diff which ignores whitespace:

$ hg diff -b
diff --git a/src/share/classes/sun/security/pkcs11/P11Signature.java
b/src/share/classes/sun/security/pkcs11/P11Signature.java
--- a/src/share/classes/sun/security/pkcs11/P11Signature.java
+++ b/src/share/classes/sun/security/pkcs11/P11Signature.java
@@ -257,6 +257,8 @@
             session = token.killSession(session);
             return;
         }
+
+        try {
         // "cancel" operation by finishing it
         // XXX make sure all this always works correctly
         if (mode == M_SIGN) {
@@ -299,6 +301,9 @@
                 // XXX check error code
             }
         }
+        } finally {
+            session = token.releaseSession(session);
+        }
     }

     // assumes current state is initialized == false
@@ -316,6 +321,8 @@
             }
             initialized = true;
         } catch (PKCS11Exception e) {
+            // release session when initialization failed
+            session = token.releaseSession(session);
             throw new ProviderException("Initialization failed", e);
         }
         if (bytesProcessed != 0) {
@@ -443,6 +450,8 @@
                 }
                 bytesProcessed += len;
             } catch (PKCS11Exception e) {
+                initialized = false;
+                session = token.releaseSession(session);
                 throw new ProviderException(e);
             }
             break;
@@ -490,6 +499,8 @@
                 bytesProcessed += len;
                 byteBuffer.position(ofs + len);
             } catch (PKCS11Exception e) {
+                initialized = false;
+                session = token.releaseSession(session);
                 throw new ProviderException("Update failed", e);
             }
             break;

Are you able to push or shall I do it on your behalf?
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Web Site: http://fuseyism.com
Twitter: https://twitter.com/gnu_andrew_java
PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222