Fix 98613: using SSLSocket after close causes crash.

Now the SSLSocket and SSLServerSocket close() methods only call PR_Close()
on the underlying fd. Freeing data structures is postponed until the
Java object is finalized.

security/jss/lib/jss.def

@@ -246,6 +246,7 @@ Java_org_mozilla_jss_CryptoManager_verifyCertNowNative;
 Java_org_mozilla_jss_ssl_SSLServerSocket_setServerCert;
 Java_org_mozilla_jss_ssl_SocketBase_setClientCert;
 Java_org_mozilla_jss_CryptoManager_verifyCertTempNative;
+Java_org_mozilla_jss_ssl_SocketProxy_releaseNativeResources;
 ;+    local:
 ;+       *;
 ;+};

security/jss/org/mozilla/jss/ssl/SSLServerSocket.java

@@ -186,9 +186,7 @@ public class SSLServerSocket extends java.net.ServerSocket {
      */
     public static native void clearSessionCache();
 
-    protected void finalize() throws Throwable {
-        close();
-    }
+    protected void finalize() throws Throwable { }
 
 
     /**
@@ -202,10 +200,7 @@ public class SSLServerSocket extends java.net.ServerSocket {
      * Closes this socket.
      */
     public void close() throws IOException {
-        if( sockProxy != null ) {
         base.close();
-            sockProxy = null;
-        }
     }
 
     // This directory is used as the default for the Session ID cache

security/jss/org/mozilla/jss/ssl/SSLSocket.java

@@ -386,10 +386,7 @@ public class SSLSocket extends java.net.Socket {
      * Closes this socket.
      */
     public void close() throws IOException {
-        if( sockProxy != null ) {
         base.close();
-            sockProxy = null;
-        }
     }
 
     private native void socketConnect(byte[] addr, String hostname, int port)
@@ -666,9 +663,7 @@ public class SSLSocket extends java.net.Socket {
      */
     public native void redoHandshake(boolean flushCache) throws SocketException;
 
-    protected void finalize() throws Throwable {
-        close();
-    }
+    protected void finalize() throws Throwable { }
 
     public static class CipherPolicy {
         private int enum;

security/jss/org/mozilla/jss/ssl/SocketBase.java

@@ -101,10 +101,11 @@ class SocketBase {
 
     void close() throws IOException {
         socketClose();
-        sockProxy = null;
     }
 
-    native void socketClose() throws IOException;
+    // This method is synchronized because there is a potential race
+    // condition in the native code.
+    native synchronized void socketClose() throws IOException;
 
     private boolean requestingClientAuth = false;
 

security/jss/org/mozilla/jss/ssl/SocketProxy.java

@@ -39,14 +39,10 @@ class SocketProxy extends org.mozilla.jss.util.NativeProxy {
         super(pointer);
     }
 
-    /** 
-     * Theoretically, we don't need to do anything, since SSLSocket should
-     * call close() when it finalizes. When a socket is closed all its
-     * resources are freed, and there's nothing left to release.
-     */
-    protected void releaseNativeResources() { }
+    protected native void releaseNativeResources();
 
     protected void finalize() throws Throwable {
+        System.out.println("In SocketProxy.finalize");
         super.finalize();
     }
 }

security/jss/org/mozilla/jss/ssl/common.c

@@ -263,6 +263,7 @@ JSSL_CreateSocketData(JNIEnv *env, jobject sockObj, PRFileDesc* newFD,
     sockdata->clientCertSelectionCallback = NULL;
     sockdata->clientCert = NULL;
     sockdata->jsockPriv = priv;
+    sockdata->closed = PR_FALSE;
 
     /*
      * Make a global ref to the socket. Since it is a weak reference, it will
@@ -285,15 +286,35 @@ finish:
     return sockdata;
 }
 
+JNIEXPORT void JNICALL
+Java_org_mozilla_jss_ssl_SocketProxy_releaseNativeResources
+    (JNIEnv *env, jobject this)
+{
+    JSSL_SocketData *sock = NULL;
+
+    /* get the FD */
+    if( JSS_getPtrFromProxy(env, this, (void**)&sock) != PR_SUCCESS) {
+        /* exception was thrown */
+        goto finish;
+    }
+
+    JSSL_DestroySocketData(env, sock);
+
+finish:
+    return;
+}
+
 void
 JSSL_DestroySocketData(JNIEnv *env, JSSL_SocketData *sd)
 {
     PR_ASSERT(sd != NULL);
 
-    if( sd->fd != NULL ) {
+    if( !sd->closed ) {
         PR_Close(sd->fd);
+        sd->closed = PR_TRUE;
         /* this may have thrown an exception */
     }
+
     if( sd->socketObject != NULL ) {
         DELETE_WEAK_GLOBAL_REF(env, sd->socketObject );
     }
@@ -375,6 +396,11 @@ finish:
     }
 }
 
+/*
+ * This method is synchronized because of a potential race condition.
+ * We want to avoid two threads simultaneously calling this code, in case
+ * one sets sd->fd to NULL and then the other calls PR_Close on the NULL.
+ */
 JNIEXPORT void JNICALL
 Java_org_mozilla_jss_ssl_SocketBase_socketClose(JNIEnv *env, jobject self)
 {
@@ -386,11 +412,14 @@ Java_org_mozilla_jss_ssl_SocketBase_socketClose(JNIEnv *env, jobject self)
         goto finish;
     }
 
-    /* destroy the FD and any supporting data */
-    JSSL_DestroySocketData(env, sock);
+    if( ! sock->closed ) {
+        PR_Close(sock->fd);
+        sock->closed = PR_TRUE;
+        /* this may have thrown an exception */
+    }
 
 finish:
-    /* Don't do EXCEPTION_CHECK, because the underlying fd has been deleted */
+    EXCEPTION_CHECK(env, sock)
     return;
 }
 

security/jss/org/mozilla/jss/ssl/jssl.h

@@ -41,6 +41,7 @@ struct JSSL_SocketData {
     jobject clientCertSelectionCallback; /* global ref */
     CERTCertificate *clientCert;
     PRFilePrivate *jsockPriv;
+    PRBool closed;
 };
 typedef struct JSSL_SocketData JSSL_SocketData;