Hi,
I've run into the following issue in an earlier version of conscrypt which I think might still apply to ConscryptEngine.java
closeOutbound()
calls closeAndFreeResources
And:
closeAndFreeResources has:
private void closeAndFreeResources() {
transitionTo(STATE_CLOSED);
if (!ssl.isClosed()) {
ssl.close();
networkBio.close();
}
}
And finally ssl's close is given by:
void close() {
NativeCrypto.SSL_free(ssl, this);
ssl = 0L;
}
The problem is that the finalizer for ssl can be called right after the call to SSL_free and about the time that ssl = 0 is happening. There's a race on ssl as I assume finalize is happening in another thread. And close and SSL_free can be called twice with the same non-null value of ssl and this can be problematic.
closeOutbound is synchronized on ssl object but the finalizer is not.
The finalizer is:
@OverRide
protected final void finalize() throws Throwable {
try {
if (!isClosed()) {
close();
}
} finally {
super.finalize();
}
}
I faced this issue in OpenSSLSocketImpl.java which has since been refactored but it's the same issue here.
The following sort of change seems to fix the issue for me:
void close() {
long tmpSsl = ssl;
ssl = 0;
NativeCrypto.SSL_free(tmpSsl, this);
}
I initially assumed that making my ssl pointer volatile (as indeed it is in NativeSsl.java) should fix my issue but in my case it does not. I assume because both threads are reading and writing the pointer value.
In my case I can reproduce the issue by running 100 clients and servers and making a large number of connections from the clients to the servers and introducing random failures in sock_read and sock_write (in the underlying boringssl/openssl), which I assume will trigger the problematic free on handshake failure. I see double free in SSL_free. In my case it's running 100 connection per client and running the test 10 times in a row. It usually fails between 1-4 times through. (so 10,000 to 40,000 connections with a failure rate of 1 in a thousand reads and writes).
Bijan
Hi,
I've run into the following issue in an earlier version of conscrypt which I think might still apply to ConscryptEngine.java
closeOutbound()
calls closeAndFreeResources
And:
closeAndFreeResources has:
private void closeAndFreeResources() {
transitionTo(STATE_CLOSED);
if (!ssl.isClosed()) {
ssl.close();
networkBio.close();
}
}
And finally ssl's close is given by:
void close() {
NativeCrypto.SSL_free(ssl, this);
ssl = 0L;
}
The problem is that the finalizer for ssl can be called right after the call to SSL_free and about the time that ssl = 0 is happening. There's a race on ssl as I assume finalize is happening in another thread. And close and SSL_free can be called twice with the same non-null value of ssl and this can be problematic.
closeOutbound is synchronized on ssl object but the finalizer is not.
The finalizer is:
@OverRide
protected final void finalize() throws Throwable {
try {
if (!isClosed()) {
close();
}
} finally {
super.finalize();
}
}
I faced this issue in OpenSSLSocketImpl.java which has since been refactored but it's the same issue here.
The following sort of change seems to fix the issue for me:
void close() {
long tmpSsl = ssl;
ssl = 0;
NativeCrypto.SSL_free(tmpSsl, this);
}
I initially assumed that making my ssl pointer volatile (as indeed it is in NativeSsl.java) should fix my issue but in my case it does not. I assume because both threads are reading and writing the pointer value.
In my case I can reproduce the issue by running 100 clients and servers and making a large number of connections from the clients to the servers and introducing random failures in sock_read and sock_write (in the underlying boringssl/openssl), which I assume will trigger the problematic free on handshake failure. I see double free in SSL_free. In my case it's running 100 connection per client and running the test 10 times in a row. It usually fails between 1-4 times through. (so 10,000 to 40,000 connections with a failure rate of 1 in a thousand reads and writes).
Bijan