ensure that shutdowns are called appropriately, and work

the BIO which has produced the fatal error calls the shutdown
routine

diff --git a/src/lib/bio/base.c b/src/lib/bio/base.c
index 1039e999c3afdf58c233a1b55beec9bc41384eca..7f2b086babc82fa38d572cb5648bd43c594737c8 100644 (file)
--- a/src/lib/bio/base.c
+++ b/src/lib/bio/base.c
@@ -135,42 +135,46 @@ int fr_bio_free(fr_bio_t *bio)
 
 /** Shut down a set of BIOs
  *
- *  Must be called from the top-most bio.
- *
- *  Will shut down the bios from the bottom-up.
- *
- *  The shutdown function MUST be callable multiple times without breaking.
+ *  We shut down the BIOs from the top to the bottom.  This gives the TLS BIO an opportunity to
+ *  call the SSL_shutdown() routine, which should then write to the FD BIO.
  */
 int fr_bio_shutdown(fr_bio_t *bio)
 {
-       fr_bio_t *last;
+       fr_bio_t *this, *first;
+       fr_bio_common_t *my;
 
        fr_assert(!fr_bio_prev(bio));
 
        /*
-        *      Find the last bio in the chain.
+        *      Find the first bio in the chain.
         */
-       for (last = bio; fr_bio_next(last) != NULL; last = fr_bio_next(last)) {
+       for (this = bio; fr_bio_prev(this) != NULL; this = fr_bio_prev(this)) {
                /* nothing */
        }
+       first = this;
 
        /*
-        *      Walk back up the chain, calling the shutdown functions.
+        *      Walk back down the chain, calling the shutdown functions.
         */
-       do {
-               fr_bio_common_t *my = (fr_bio_common_t *) last;
+       for (/* nothing */; this != NULL; this = fr_bio_next(this)) {
+               my = (fr_bio_common_t *) this;
+
+               if (!my->priv_cb.shutdown) continue;
 
                /*
-                *      Call user shutdown before the bio shutdown.
-                *
-                *      Then set it to NULL so that it doesn't get called again on talloc cleanups.
+                *      The EOF handler said it's NOT at EOF, so we stop processing here.
                 */
-               if (my->cb.shutdown) my->cb.shutdown(last);
+               my->priv_cb.shutdown(&my->bio);
+               my->priv_cb.shutdown = NULL;
+       }
 
-               my->cb.shutdown = NULL;
+       /*
+        *      Call the application shutdown routine
+        */
+       my = (fr_bio_common_t *) first;
 
-               last = fr_bio_prev(last);
-       } while (last);
+       if (my->cb.shutdown) my->cb.shutdown(first);
+       my->cb.shutdown = NULL;
 
        return 0;
 }

diff --git a/src/lib/bio/fd_errno.h b/src/lib/bio/fd_errno.h
index 1fd15a84800ecdfdc5b4c3e59569122e435585c6..0b52a2cd924f2a812bc86a01f7dd03cd8e0735f7 100644 (file)
--- a/src/lib/bio/fd_errno.h
+++ b/src/lib/bio/fd_errno.h
@@ -52,3 +52,8 @@ default:
         */
        break;
 }
+
+/*
+ *     Shut down the BIO.  It's no longer useable.
+ */
+fr_bio_shutdown(&my->bio);

diff --git a/src/lib/bio/mem.c b/src/lib/bio/mem.c
index 0a89735a050cda7a6fa9e4979d9a54440f618c2b..c66df4f187d40d34e23024b6dd53cf5529c3df32 100644 (file)
--- a/src/lib/bio/mem.c
+++ b/src/lib/bio/mem.c
@@ -491,19 +491,15 @@ static ssize_t fr_bio_mem_write_flush(fr_bio_mem_t *my, size_t size)
        rcode = next->write(next, NULL, my->write_buffer.write, used);
 
        /*
-        *      The next bio returned an error.  Anything other than WOULD BLOCK is fatal.  We can read from
-        *      the memory buffer until it's empty, but we can no longer write to the memory buffer.
+        *      We didn't write anything, the bio is blocked.
         */
-       if ((rcode < 0) && (rcode != fr_bio_error(IO_WOULD_BLOCK))) {
-               my->bio.read = fr_bio_mem_read_eof;
-               my->bio.write = fr_bio_null_write;
-               return rcode;
-       }
+       if ((rcode == 0) || (rcode == fr_bio_error(IO_WOULD_BLOCK))) return fr_bio_error(IO_WOULD_BLOCK);
 
        /*
-        *      We didn't write anything, the bio is still blocked.
+        *      All other errors are fatal.  We can read from the memory buffer until it's empty, but we can
+        *      no longer write to the memory buffer.
         */
-       if ((rcode == 0) || (rcode == fr_bio_error(IO_WOULD_BLOCK))) return fr_bio_error(IO_WOULD_BLOCK);
+       if (rcode < 0) return rcode;
 
        /*
         *      Tell the buffer that we've read a certain amount of data from it.
@@ -682,6 +678,15 @@ static int fr_bio_mem_call_verify(fr_bio_t *bio, void *packet_ctx, size_t *size)
        return -1;
 }
 
+/*
+ *     The application can read from the BIO until EOF, but cannot write to it.
+ */
+static void fr_bio_mem_shutdown(fr_bio_t *bio)
+{
+       bio->read = fr_bio_mem_read_eof;
+       bio->write = fr_bio_null_write;
+}
+
 /** Allocate a memory buffer bio for either reading or writing.
  */
 static bool fr_bio_mem_buf_alloc(fr_bio_mem_t *my, fr_bio_buf_t *buf, size_t size)
@@ -750,6 +755,7 @@ fr_bio_t *fr_bio_mem_alloc(TALLOC_CTX *ctx, size_t read_size, size_t write_size,
        }
        my->priv_cb.eof = fr_bio_mem_eof;
        my->priv_cb.write_resume = fr_bio_mem_write_resume;
+       my->priv_cb.shutdown = fr_bio_mem_shutdown;
 
        fr_bio_chain(&my->bio, next);
 
@@ -783,6 +789,11 @@ fr_bio_t *fr_bio_mem_source_alloc(TALLOC_CTX *ctx, size_t write_size, fr_bio_t *
        my->bio.read = fr_bio_null_read; /* reading FROM this bio is not possible */
        my->bio.write = fr_bio_mem_write_next;
 
+       /*
+        *      @todo - have write pause / write resume callbacks?
+        */
+       my->priv_cb.shutdown = fr_bio_mem_shutdown;
+
        fr_bio_chain(&my->bio, next);
 
        talloc_set_destructor((fr_bio_t *) my, fr_bio_destructor);
@@ -829,7 +840,7 @@ static ssize_t fr_bio_mem_write_read_buffer(fr_bio_t *bio, UNUSED void *packet_c
 
 /** Allocate a memory buffer which sinks data from a bio system into the callers application.
  *
- *  The caller reads data from this bio, but never writes to it.  Upstream bios will source the data.
+ *  The caller reads data from this bio, but never writes to it.  Upstream BIOs will source the data.
  */
 fr_bio_t *fr_bio_mem_sink_alloc(TALLOC_CTX *ctx, size_t read_size)
 {

diff --git a/src/lib/bio/retry.c b/src/lib/bio/retry.c
index d86a01902d1edc839da2927c1513cf7cee5c4e64..a0d3078b8bb4db8ba2a96e7b0d3ef85a54c66f4b 100644 (file)
--- a/src/lib/bio/retry.c
+++ b/src/lib/bio/retry.c
@@ -1071,6 +1071,16 @@ static int fr_bio_retry_destructor(fr_bio_retry_t *my)
        return 0;
 }
 
+/**  Orderly shutdown.
+ *
+ */
+static void fr_bio_retry_shutdown(fr_bio_t *bio)
+{
+       fr_bio_retry_t *my = talloc_get_type_abort(bio, fr_bio_retry_t);
+
+       (void) fr_bio_retry_destructor(my);
+}
+
 /**  Allocate a #fr_bio_retry_t
  *
  */
@@ -1136,6 +1146,7 @@ fr_bio_t *fr_bio_retry_alloc(TALLOC_CTX *ctx, size_t max_saved,
 
        my->priv_cb.write_blocked = fr_bio_retry_write_blocked;
        my->priv_cb.write_resume = fr_bio_retry_write_resume;
+       my->priv_cb.shutdown = fr_bio_retry_shutdown;
 
        fr_bio_chain(&my->bio, next);