From: Andrew Deason <adeason@sinenomine.net>
Date: Wed, 28 Oct 2009 16:06:47 +0000 (-0500)
Subject: Avoid using released hosts
X-Git-Tag: debian/1.4.11+dfsg-5~7
X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=b4f047ffd28cb76a4afbca65cd7b8316d49a3d59;p=packages%2Fo%2Fopenafs.git

Avoid using released hosts

Since h_Release_r has the possibility of freeing a host, we should not
be using a host after it has been released. A few places can still use a
released host, potentially causing heap corruption, double frees, and
generally weird behavior.

So either move calls of h_Release_r until after we finish using a host,
or make sure to set the pointer to NULL after it has been released.

Reviewed-on: http://gerrit.openafs.org/747
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Dan Hyde <drh@umich.edu>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit 416e2f11c35f5d55f91090b30b4db1a9bf6d6e07)

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

diff --git a/src/viced/afsfileprocs.c b/src/viced/afsfileprocs.c
index 638a8238e..9695cf714 100644
--- a/src/viced/afsfileprocs.c
+++ b/src/viced/afsfileprocs.c
@@ -429,9 +429,6 @@ CallPostamble(register struct rx_connection *aconn, afs_int32 ret,
     if (thost->hostFlags & HERRORTRANS)
 	translate = 1;
     h_ReleaseClient_r(tclient);
-    held = h_Held_r(thost);
-    if (held)
-	h_Release_r(thost);
     if (ahost && ahost != thost) {
 	char hoststr[16], hoststr2[16];	
 	ViceLog(0, ("CallPostamble: ahost %s:%d (%x) != thost %s:%d (%x)\n",
@@ -446,6 +443,9 @@ CallPostamble(register struct rx_connection *aconn, afs_int32 ret,
 		afs_inet_ntoa_r(thost->host, hoststr), ntohs(thost->port),
 		thost));
     }
+    held = h_Held_r(thost);
+    if (held)
+	h_Release_r(thost);
  busyout:
     H_UNLOCK;
     return (translate ? sys_error_to_et(ret) : ret);
diff --git a/src/viced/callback.c b/src/viced/callback.c
index 70ac594ec..e8b5a40d1 100644
--- a/src/viced/callback.c
+++ b/src/viced/callback.c
@@ -1672,8 +1672,10 @@ GetSomeSpace_r(struct host *hostp, int locked)
 		    h_Release_r(hp);
 		return 0;
 	    }
-	    if (lih_host_held2)
+	    if (lih_host_held2) {
 		h_Release_r(hp);
+		hp = NULL;
+	    }
 	    hp1 = hp;
 	    hp2 = hostList;
 	} else {
diff --git a/src/viced/host.c b/src/viced/host.c
index 7033e251e..4729774c0 100644
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -852,6 +852,7 @@ h_Lookup_r(afs_uint32 haddr, afs_uint16 hport, int *heldp, struct host **hostp)
 		h_Unlock_r(host);
 		if (!*heldp)
 		    h_Release_r(host);
+		host = NULL;
 		goto restart;
 	    }
 	    h_Unlock_r(host);
@@ -1458,12 +1459,12 @@ h_GetHost_r(struct rx_connection *tcon)
 	if (!(host->hostFlags & ALTADDR)) {
 	    /* Another thread is doing initialization */
 	    h_Unlock_r(host);
-	    if (!held)
-		h_Release_r(host);
 	    ViceLog(125,
 		    ("Host %s:%d starting h_Lookup again\n",
 		     afs_inet_ntoa_r(host->host, hoststr),
 		     ntohs(host->port)));
+	    if (!held)
+		h_Release_r(host);
 	    goto retry;
 	}
 	host->hostFlags |= HWHO_INPROGRESS;
@@ -1687,12 +1688,12 @@ h_GetHost_r(struct rx_connection *tcon)
 		     ntohs(host->port)));
 	    h_Lock_r(host);
 	    h_Unlock_r(host);
-	    if (!held)
-		h_Release_r(host);
 	    ViceLog(125,
 		    ("Host %s:%d starting h_Lookup again\n",
 		     afs_inet_ntoa_r(host->host, hoststr),
 		     ntohs(host->port)));
+	    if (!held)
+		h_Release_r(host);
 	    goto retry;
 	}
 	/* We need to check whether the identity in the host structure