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.

Change-Id: I3d5275c3862003e372d3c19a5462e62bf9cb269e
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>

diff --git a/src/viced/afsfileprocs.c b/src/viced/afsfileprocs.c
index 5041b4f..3cad9c7 100644 (file)
--- a/src/viced/afsfileprocs.c
+++ b/src/viced/afsfileprocs.c
@@ -430,10 +430,6 @@ CallPostamble(register struct rx_connection *aconn, afs_int32 ret,
        translate = 1;
     h_ReleaseClient_r(tclient);
 
-    /* return the reference taken in local h_FindClient_r--h_ReleaseClient_r
-     * does not decrement refcount on client->host */
-    h_Release_r(thost);
-
     if (ahost) {
            if (ahost != thost) {
                    /* host/client recycle */
@@ -457,6 +453,10 @@ CallPostamble(register struct rx_connection *aconn, afs_int32 ret,
                        thost));
     }
 
+    /* return the reference taken in local h_FindClient_r--h_ReleaseClient_r
+     * does not decrement refcount on client->host */
+    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 0403221..26f1bb2 100644 (file)
--- a/src/viced/callback.c
+++ b/src/viced/callback.c
@@ -1568,8 +1568,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 4c2de65..9bd3b6e 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -815,6 +815,7 @@ h_Lookup_r(afs_uint32 haddr, afs_uint16 hport, struct host **hostp)
            if (host->hostFlags & HOSTDELETED) {
                h_Unlock_r(host);
                h_Release_r(host);
+               host = NULL;
                goto restart;
            }
            h_Unlock_r(host);
@@ -1503,11 +1504,11 @@ h_GetHost_r(struct rx_connection *tcon)
        if (!(host->hostFlags & ALTADDR)) {
            /* Another thread is doing initialization */
            h_Unlock_r(host);
-           h_Release_r(host);
            ViceLog(125,
                    ("Host %" AFS_PTR_FMT " (%s:%d) starting h_Lookup again\n",
                     host, afs_inet_ntoa_r(host->host, hoststr),
                     ntohs(host->port)));
+           h_Release_r(host);
            goto retry;
        }
        host->hostFlags |= HWHO_INPROGRESS;
@@ -1728,11 +1729,11 @@ h_GetHost_r(struct rx_connection *tcon)
                     ntohs(host->port)));
            h_Lock_r(host);
            h_Unlock_r(host);
-           h_Release_r(host);
            ViceLog(125,
                    ("Host %" AFS_PTR_FMT " (%s:%d) starting h_Lookup again\n",
                     host, afs_inet_ntoa_r(host->host, hoststr),
                     ntohs(host->port)));
+           h_Release_r(host);
            goto retry;
        }
        /* We need to check whether the identity in the host structure