From ae6a3c79366d3906f46f59f424f2b9ef72a8186e Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Thu, 22 Oct 2009 11:12:30 -0500 Subject: [PATCH] Avoid prematurely destroying callback_rxcon Currently, h_GetHost_r and removeAddress_r can destroy the callback_rxcon of a host. Having a NULL callback_rxcon can cause segfaults in code that does not properly check if a host has been HOSTDELETED before trying to use it. Although such code is incorrect and should be fixed, we can still avoid a segfault in those situations by not destroying callback_rxcon until we destroy the host itself. This just prevents destroying callback_rxcon in h_GetHost_r and removeAddress_r, leaving it to h_TossStuff_r to destroy when it destroys the host. Reviewed-on: http://gerrit.openafs.org/717 Tested-by: Andrew Deason Reviewed-by: Jeffrey Altman Tested-by: Derrick Brashear Reviewed-by: Derrick Brashear (cherry picked from commit bbcfbe1a04eda9e75b1643be88cf9d4842a8aa86) Change-Id: I4d44fca9b79b656bc70f8108616f23f2dfaa353f Reviewed-on: http://gerrit.openafs.org/749 Tested-by: Andrew Deason Reviewed-by: Derrick Brashear --- src/viced/host.c | 52 ++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/src/viced/host.c b/src/viced/host.c index 6dbb4f4cb..7033e251e 100644 --- a/src/viced/host.c +++ b/src/viced/host.c @@ -1334,23 +1334,6 @@ removeAddress_r(struct host *host, afs_uint32 addr, afs_uint16 port) host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port))); host->hostFlags |= HOSTDELETED; } else { - struct rx_connection *rxconn; - - rxconn = host->callback_rxcon; - host->callback_rxcon = NULL; - - if (rxconn) { - struct client *client; - /* - * If rx_DestroyConnection calls h_FreeConnection we will - * deadlock on the host_glock_mutex. Work around the problem - * by unhooking the client from the connection before - * destroying the connection. - */ - client = rx_GetSpecific(rxconn, rxcon_client_key); - rx_SetSpecific(rxconn, rxcon_client_key, (void *)0); - rx_DestroyConnection(rxconn); - } for (i=0; i < host->interface->numberOfInterfaces; i++) { if (host->interface->interface[i].valid) { @@ -1372,6 +1355,25 @@ removeAddress_r(struct host *host, afs_uint32 addr, afs_uint16 port) host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port))); host->hostFlags |= HOSTDELETED; } else { + struct rx_connection *rxconn; + + rxconn = host->callback_rxcon; + host->callback_rxcon = NULL; + + if (rxconn) { + struct client *client; + /* + * If rx_DestroyConnection calls h_FreeConnection we will + * deadlock on the host_glock_mutex. Work around the problem + * by unhooking the client from the connection before + * destroying the connection. + */ + client = rx_GetSpecific(rxconn, rxcon_client_key); + rx_SetSpecific(rxconn, rxcon_client_key, (void *)0); + rx_DestroyConnection(rxconn); + rxconn = NULL; + } + if (!sc) sc = rxnull_NewClientSecurityObject(); host->callback_rxcon = @@ -1876,20 +1878,10 @@ h_GetHost_r(struct rx_connection *tcon) oldHost->port = hport; rxconn = oldHost->callback_rxcon; oldHost->callback_rxcon = host->callback_rxcon; - host->callback_rxcon = NULL; + host->callback_rxcon = rxconn; - if (rxconn) { - struct client *client; - /* - * If rx_DestroyConnection calls h_FreeConnection we will - * deadlock on the host_glock_mutex. Work around the problem - * by unhooking the client from the connection before - * destroying the connection. - */ - client = rx_GetSpecific(rxconn, rxcon_client_key); - rx_SetSpecific(rxconn, rxcon_client_key, (void *)0); - rx_DestroyConnection(rxconn); - } + /* don't destroy rxconn here; let h_TossStuff_r + * take care of that via h_Release_r below */ } host->hostFlags &= ~HWHO_INPROGRESS; h_Unlock_r(host); -- 2.39.5