From: Alain Spineux <alain@baculasystems.com>
Date: Thu, 22 Nov 2018 18:05:21 +0000 (+0100)
Subject: fix #4383 Sometime SD hangs when TLS and DEDUP are used together
X-Git-Tag: Release-9.4.3~60
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c9b1a499f93a491e096d3c7e0079043e2137c99a;p=thirdparty%2Fbacula.git

fix #4383 Sometime SD hangs when TLS and DEDUP are used together

- the problem is that with DEDUP 2 threads use the BSOCK in //,
  and our TLS layer was not ready for that.
  - the bsock->set_nonblocking() and restore_blocking() were not protected
    by the mutex
  - SSL_get_error() was not inside the mutex
- the code inside the mutex keep sending until everything is sent or
  there is an error condition.
- notice a small change in the code, SSL_get_error() is only called
  when nwritten<=0
- I think this could also happens on the FD even when DEDUP is not used,
  just because of the heartbeat

Signed-off-by: Alain Spineux <alain@baculasystems.com>
---

diff --git a/bacula/src/lib/tls.c b/bacula/src/lib/tls.c
index 604ccaee7..8e32dfbd2 100644
--- a/bacula/src/lib/tls.c
+++ b/bacula/src/lib/tls.c
@@ -627,12 +627,9 @@ void tls_bsock_shutdown(BSOCKCORE *bsock)
 static inline int openssl_bsock_readwrite(BSOCK *bsock, char *ptr, int nbytes, bool write)
 {
    TLS_CONNECTION *tls = bsock->tls;
-   int flags;
    int nleft = 0;
    int nwritten = 0;
 
-   /* Ensure that socket is non-blocking */
-   flags = bsock->set_nonblocking();
 
    /* start timer */
    bsock->timer_start = watchdog_time;
@@ -647,20 +644,32 @@ static inline int openssl_bsock_readwrite(BSOCK *bsock, char *ptr, int nbytes, b
    while (nleft > 0) {
 
       pthread_mutex_lock(&tls->rwlock);
-      if (write) {
-         nwritten = SSL_write(tls->openssl, ptr, nleft);
-      } else {
-         nwritten = SSL_read(tls->openssl, ptr, nleft);
+      /* Ensure that socket is non-blocking */
+      int flags = bsock->set_nonblocking();
+      int ssl_error = SSL_ERROR_NONE;
+      while (nleft > 0 && ssl_error == SSL_ERROR_NONE) {
+         if (write) {
+            nwritten = SSL_write(tls->openssl, ptr, nleft);
+         } else {
+            nwritten = SSL_read(tls->openssl, ptr, nleft);
+         }
+         if (nwritten > 0) {
+            nleft -= nwritten;
+            if (nleft) {
+               ptr += nwritten;
+            }
+         } else {
+            ssl_error = SSL_get_error(tls->openssl, nwritten);
+         }
       }
+      /* Restore saved flags */
+      bsock->restore_blocking(flags);
       pthread_mutex_unlock(&tls->rwlock);
 
       /* Handle errors */
-      switch (SSL_get_error(tls->openssl, nwritten)) {
+      switch (ssl_error) {
       case SSL_ERROR_NONE:
-         nleft -= nwritten;
-         if (nleft) {
-            ptr += nwritten;
-         }
+         ASSERT2(nleft == 0, "the buffer should be empty");
          break;
 
       case SSL_ERROR_SYSCALL:
@@ -710,8 +719,6 @@ cleanup:
    if (write) {
       pthread_mutex_unlock(&tls->wlock);
    }
-   /* Restore saved flags */
-   bsock->restore_blocking(flags);
 
    /* Clear timer */
    bsock->timer_start = 0;