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 f9080527be106218e383b312871ffced7a5bb68f..5becb0195100d377f32e892a63438b1151ef9d27 100644 (file)
--- a/src/viced/afsfileprocs.c
+++ b/src/viced/afsfileprocs.c
@@ -427,9 +427,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",
@@ -444,6 +441,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 b6679951fb9499189ee017f0605215f27fb91a94..ca19bd49079cf09d4ae87ff655df70eecd41e761 100644 (file)
--- a/src/viced/callback.c
+++ b/src/viced/callback.c
@@ -1670,8 +1670,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 dca3042cd39db3e05a547ed68c9a3396e1f1f09b..e28f79a98c8d3e84c66e27403236413851e1b14e 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -850,6 +850,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);
@@ -1456,12 +1457,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;
@@ -1685,12 +1686,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