From: Alan T. DeKok Date: Wed, 20 Nov 2024 20:25:23 +0000 (-0500) Subject: ensure that shutdowns are called appropriately, and work X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6d034298d0e2cd8045e2d358a4edf5e764e3b83f;p=thirdparty%2Ffreeradius-server.git 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 1039e999c3a..7f2b086babc 100644 --- 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 1fd15a84800..0b52a2cd924 100644 --- 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 0a89735a050..c66df4f187d 100644 --- 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 d86a01902d1..a0d3078b8bb 100644 --- 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);