From 38963e759643e05836304fbf2b3b45c6c8db3e3a Mon Sep 17 00:00:00 2001 From: Kyle Ambroff-Kao Date: Wed, 27 May 2020 20:40:45 -0700 Subject: [PATCH] SecureSocket: Handle interrupted reads and writes If a system call performed by SSL_read is interrupted by a signal, it seems to set its error to SSL_ERROR_WANT_READ. This triggers logic added in hrev53853 which assumes the caller is doing async reads and returns B_WOULD_BLOCK. This breaks uses of BSecureSocket that do blocking reads. * Detect interrupted signal by checking for EINTR in errno. * Adding this retry loop to BScureSocket::Write as well since it can have the same problem. Resolves issue #15853. Change-Id: I8198a8496fa3a2ccee00bda87375a482a0d4ba3d Reviewed-on: https://review.haiku-os.org/c/haiku/+/2825 Reviewed-by: Adrien Destugues --- src/kits/network/libnetapi/SecureSocket.cpp | 45 ++++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/kits/network/libnetapi/SecureSocket.cpp b/src/kits/network/libnetapi/SecureSocket.cpp index 5e1539c190..31f8115d7e 100644 --- a/src/kits/network/libnetapi/SecureSocket.cpp +++ b/src/kits/network/libnetapi/SecureSocket.cpp @@ -549,14 +549,21 @@ BSecureSocket::Read(void* buffer, size_t size) bytesRead = SSL_read(fPrivate->fSSL, buffer, size); if (bytesRead >= 0) return bytesRead; - // Don't retry in cases of "no data available" for non-blocking sockets - int error = SSL_get_error(fPrivate->fSSL, bytesRead); - if (error == SSL_ERROR_WANT_READ || error == SSL_ERROR_WANT_WRITE) - return B_WOULD_BLOCK; - // Otherwise, check if we should retry (maybe we were interrupted by - // a signal, for example) + + if (errno != EINTR) { + // Don't retry in cases of "no data available" for non-blocking + // sockets. + int error = SSL_get_error(fPrivate->fSSL, bytesRead); + if (error == SSL_ERROR_WANT_READ || error == SSL_ERROR_WANT_WRITE) + return B_WOULD_BLOCK; + } + + // See if the error was retryable. We may have been interrupted by + // a signal, in which case we will retry. But it is also possible that + // another error has occurred which is not retryable. openssl will + // decide for us here. retry = BIO_should_retry(SSL_get_rbio(fPrivate->fSSL)); - } while(retry != 0); + } while (retry != 0); return fPrivate->ErrorCode(bytesRead); } @@ -568,9 +575,27 @@ BSecureSocket::Write(const void* buffer, size_t size) if (!IsConnected()) return B_ERROR; - int bytesWritten = SSL_write(fPrivate->fSSL, buffer, size); - if (bytesWritten >= 0) - return bytesWritten; + int bytesWritten; + int retry; + do { + bytesWritten = SSL_write(fPrivate->fSSL, buffer, size); + if (bytesWritten >= 0) + return bytesWritten; + + if (errno != EINTR) { + // Don't retry in cases of "no buffer space available" for + // non-blocking sockets. + int error = SSL_get_error(fPrivate->fSSL, bytesWritten); + if (error == SSL_ERROR_WANT_READ || error == SSL_ERROR_WANT_WRITE) + return B_WOULD_BLOCK; + } + + // See if the error was retryable. We may have been interrupted by + // a signal, in which case we will retry. But it is also possible that + // another error has occurred which is not retryable. openssl will + // decide for us here. + retry = BIO_should_retry(SSL_get_wbio(fPrivate->fSSL)); + } while (retry != 0); return fPrivate->ErrorCode(bytesWritten); }