2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-31 14:35:26 +00:00

Defer control channel message invalidation

The conn_shutdown() function is called whenever a control channel
connection is supposed to be closed, e.g. after a response to the client
is sent or when named is being shut down.  That function calls
isccc_ccmsg_invalidate(), which resets the magic number in the structure
holding the messages exchanged over a given control channel connection
(isccc_ccmsg_t).  The expectation here is that all operations related to
the given control channel connection will have been completed by the
time the connection needs to be shut down.

However, if named shutdown is initiated while a control channel message
is still in flight, some netmgr callbacks might still be pending when
conn_shutdown() is called and isccc_ccmsg_t invalidated.  This causes
the REQUIRE assertion checking the magic number in ccmsg_senddone() to
fail when the latter function is eventually called, resulting in a
crash.

Fix by splitting up isccc_ccmsg_invalidate() into two separate
functions:

  - isccc_ccmsg_disconnect(), which initiates TCP connection shutdown,
  - isccc_ccmsg_invalidate(), which cleans up magic number and buffer,

and then:

  - replacing all existing uses of isccc_ccmsg_invalidate() with calls
    to isccc_ccmsg_disconnect(),

  - only calling isccc_ccmsg_invalidate() when all netmgr callbacks are
    guaranteed to have been run.

Adjust function comments accordingly.
This commit is contained in:
Mark Andrews
2024-01-10 14:35:36 +11:00
committed by Michał Kępień
parent 57166cd3b7
commit d5103b742b
4 changed files with 31 additions and 14 deletions

View File

@@ -419,10 +419,10 @@ conn_shutdown(controlconnection_t *conn) {
conn->shuttingdown = true;
/*
* Calling invalidate on ccmsg will shutdown the TCP connection, thus
* we are making sure that no read callback will be called ever again.
* Close the TCP connection to make sure that no read callback will be
* called for it ever again.
*/
isccc_ccmsg_invalidate(&conn->ccmsg);
isccc_ccmsg_disconnect(&conn->ccmsg);
/* Detach the reading reference */
controlconnection_detach(&conn);
@@ -575,6 +575,8 @@ conn_free(controlconnection_t *conn) {
controllistener_t *listener = conn->listener;
isccc_ccmsg_invalidate(&conn->ccmsg);
conn_cleanup(conn);
if (conn->buffer != NULL) {

View File

@@ -348,7 +348,7 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
isccc_sexpr_free(&response);
isccc_ccmsg_invalidate(ccmsg);
isccc_ccmsg_disconnect(ccmsg);
isc_loopmgr_shutdown(loopmgr);
}
@@ -1003,6 +1003,8 @@ main(int argc, char **argv) {
isc_loopmgr_run(loopmgr);
isccc_ccmsg_invalidate(&rndc_ccmsg);
isc_log_destroy(&log);
isc_log_setcontext(NULL);

View File

@@ -179,6 +179,16 @@ isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region,
isc_nm_send(ccmsg->handle, region, ccmsg_senddone, ccmsg);
}
void
isccc_ccmsg_disconnect(isccc_ccmsg_t *ccmsg) {
REQUIRE(VALID_CCMSG(ccmsg));
if (ccmsg->handle != NULL) {
isc_nmhandle_close(ccmsg->handle);
isc_nmhandle_detach(&ccmsg->handle);
}
}
void
isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) {
REQUIRE(VALID_CCMSG(ccmsg));
@@ -188,10 +198,6 @@ isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) {
if (ccmsg->buffer != NULL) {
isc_buffer_free(&ccmsg->buffer);
}
if (ccmsg->handle != NULL) {
isc_nmhandle_close(ccmsg->handle);
isc_nmhandle_detach(&ccmsg->handle);
}
}
void

View File

@@ -120,18 +120,25 @@ isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region,
*/
void
isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg);
isccc_ccmsg_disconnect(isccc_ccmsg_t *ccmsg);
/*%
* Clean up all allocated state, and invalidate the structure.
* Disconnect from the connected netmgr handle associated with a command
* channel message.
*
* Requires:
*
*\li "ccmsg" be valid.
*\li "ccmsg" to be valid.
*/
void
isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg);
/*%
* Clean up the magic number and the dynamic buffer associated with a command
* channel message.
*
* Ensures:
* Requires:
*
*\li "ccmsg" is invalidated and disassociated with all memory contexts,
* sockets, etc.
*\li "ccmsg" to be valid.
*/
void