From: Andrew Deason <adeason@sinenomine.net>
Date: Thu, 22 Oct 2009 16:12:30 +0000 (-0500)
Subject: Avoid prematurely destroying callback_rxcon
X-Git-Tag: debian/1.4.11+dfsg-5~11
X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=ae6a3c79366d3906f46f59f424f2b9ef72a8186e;p=packages%2Fo%2Fopenafs.git

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 <adeason@sinenomine.net>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Derrick Brashear <shadow@dementia.org>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit bbcfbe1a04eda9e75b1643be88cf9d4842a8aa86)

Change-Id: I4d44fca9b79b656bc70f8108616f23f2dfaa353f
Reviewed-on: http://gerrit.openafs.org/749
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
---

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);