Check for HOSTDELETED before h_Hold_r

A few places h_Hold_r a host and later drop and reacquire H_LOCK without
checking if the hostFlags contains HOSTDELETED. This can cause a race
with h_TossStuff_r where we later reference a host that is about to be
freed or already has been freed.

Add checks for HOSTDELETED in these places, and skip over the deleted
hosts.

FIXES 126454

Reviewed-on: http://gerrit.openafs.org/1305
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit b8c3c6add90ea3face9a16ff04a1024be3d8f32d)

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

diff --git a/src/viced/callback.c b/src/viced/callback.c
index 4f351c0d178d8a6606dc160351819cc1b2236edf..4b5d04d31ffb4979ea90a135c781bd12afc910b5 100644 (file)
--- a/src/viced/callback.c
+++ b/src/viced/callback.c
@@ -940,10 +940,12 @@ BreakCallBack(struct host *xhost, AFSFid * fid, int flag)
                             ntohs(thishost->port)));
                    cb->status = CB_DELAYED;
                } else {
-                   h_Hold_r(thishost);
-                   cba[ncbas].hp = thishost;
-                   cba[ncbas].thead = cb->thead;
-                   ncbas++;
+                   if (!(thishost->hostFlags & HOSTDELETED)) {
+                       h_Hold_r(thishost);
+                       cba[ncbas].hp = thishost;
+                       cba[ncbas].thead = cb->thead;
+                       ncbas++;
+                   }
                    TDel(cb);
                    HDel(cb);
                    CDel(cb, 1);        /* Usually first; so this delete 
@@ -1324,11 +1326,14 @@ BreakVolumeCallBacks(afs_uint32 volume)
                register struct CallBack *cbnext;
                for (cb = itocb(fe->firstcb); cb; cb = cbnext) {
                    host = h_itoh(cb->hhead);
-                   h_Hold_r(host);
-                   cbnext = itocb(cb->cnext);
-                   if (!tthead || (TNorm(tthead) < TNorm(cb->thead))) {
-                       tthead = cb->thead;
+
+                   if (!(host->hostFlags & HOSTDELETED)) {
+                       h_Hold_r(host);
+                       if (!tthead || (TNorm(tthead) < TNorm(cb->thead))) {
+                           tthead = cb->thead;
+                       }
                    }
+                   cbnext = itocb(cb->cnext);
                    TDel(cb);
                    HDel(cb);
                    FreeCB(cb);
@@ -1475,9 +1480,11 @@ BreakLaterCallBacks(void)
            cbnext = itocb(cb->cnext);
            host = h_itoh(cb->hhead);
            if (cb->status == CB_DELAYED) {
-               h_Hold_r(host);
-               if (!tthead || (TNorm(tthead) < TNorm(cb->thead))) {
-                   tthead = cb->thead;
+               if (!(host->hostFlags & HOSTDELETED)) {
+                   h_Hold_r(host);
+                   if (!tthead || (TNorm(tthead) < TNorm(cb->thead))) {
+                       tthead = cb->thead;
+                   }
                }
                TDel(cb);
                HDel(cb);
@@ -1727,6 +1734,14 @@ ClearHostCallbacks_r(struct host *hp, int locked)
     ViceLog(5,
            ("GSS: Delete longest inactive host %s\n",
             afs_inet_ntoa_r(hp->host, hoststr)));
+
+    if ((hp->hostFlags & HOSTDELETED)) {
+       /* hp could go away after reacquiring H_LOCK in h_NBLock_r, so we can't
+        * really use it; its callbacks will get cleared anyway when
+        * h_TossStuff_r gets its hands on it */
+       return 1;
+    }
+
     if (!(held = h_Held_r(hp)))
        h_Hold_r(hp);
 

diff --git a/src/viced/host.c b/src/viced/host.c
index efd7e5f6966080d5b839f9b59ea5cd03ebc0cda3..77f66c9aff1b87f87fefaabec3303ddc53f9fe26 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -1054,6 +1054,7 @@ h_Enumerate(int (*proc) (), char *param)
     register struct host *host, **list;
     register int *held;
     register int i, count;
+    int totalCount;
 
     H_LOCK;
     if (hostCount == 0) {
@@ -1070,13 +1071,19 @@ h_Enumerate(int (*proc) (), char *param)
        ViceLog(0, ("Failed malloc in h_Enumerate\n"));
        assert(0);
     }
-    for (count = 0, host = hostList; host && count < hostCount; host = host->next, count++) {
-       list[count] = host;
-       if (!(held[count] = h_Held_r(host)))
-           h_Hold_r(host);
+    for (totalCount = count = 0, host = hostList;
+         host && totalCount < hostCount;
+         host = host->next, totalCount++) {
+
+       if (!(host->hostFlags & HOSTDELETED)) {
+           list[count] = host;
+           if (!(held[count] = h_Held_r(host)))
+               h_Hold_r(host);
+           count++;
+       }
     }
-    if (count != hostCount) {
-       ViceLog(0, ("h_Enumerate found %d of %d hosts\n", count, hostCount));
+    if (totalCount != hostCount) {
+       ViceLog(0, ("h_Enumerate found %d of %d hosts\n", totalCount, hostCount));
     } else if (host != NULL) {
        ViceLog(0, ("h_Enumerate found more than %d hosts\n", hostCount));
        ShutDownAndCore(PANIC);
@@ -1117,14 +1124,52 @@ h_Enumerate_r(int (*proc) (), struct host *enumstart, char *param)
     if (hostCount == 0) {
        return;
     }
+
+    host = enumstart;
+    enumstart = NULL;
+
+    /* find the first non-deleted host, so we know where to actually start
+     * enumerating */
+    for (count = 0; host && count < hostCount; count++) {
+       if (!(host->hostFlags & HOSTDELETED)) {
+           enumstart = host;
+           break;
+       }
+       host = host->next;
+    }
+    if (!enumstart) {
+       /* we didn't find a non-deleted host... */
+
+       if (host && count >= hostCount) {
+           /* ...because we found a loop */
+           ViceLog(0, ("h_Enumerate_r found more than %d hosts\n", hostCount));
+           ShutDownAndCore(PANIC);
+       }
+
+       /* ...because the hostList is full of deleted hosts */
+       return;
+    }
+
     if (enumstart && !(held = h_Held_r(enumstart)))
        h_Hold_r(enumstart); 
+
     /* remember hostCount, lest it change over the potential H_LOCK drop in
      * h_Release_r */
     origHostCount = hostCount;
 
     for (count = 0, host = enumstart; host && count < origHostCount; host = next, held = nheld, count++) {
        next = host->next;
+
+       /* find the next non-deleted host */
+       while (next && (next->hostFlags & HOSTDELETED)) {
+           next = next->next;
+           /* inc count for the skipped-over host */
+           if (++count > origHostCount) {
+               ViceLog(0, ("h_Enumerate_r found more than %d hosts\n", origHostCount));
+               ShutDownAndCore(PANIC);
+           }
+       }
+
        if (next && !(nheld = h_Held_r(next)))
            h_Hold_r(next);
        held = (*proc) (host, held, param);
@@ -2170,7 +2215,9 @@ h_FindClient_r(struct rx_connection *tcon)
 
     client = (struct client *)rx_GetSpecific(tcon, rxcon_client_key);
     if (client && client->sid == rxr_CidOf(tcon) 
-       && client->VenusEpoch == rxr_GetEpoch(tcon)) {
+       && client->VenusEpoch == rxr_GetEpoch(tcon)
+       && !(client->host->hostFlags & HOSTDELETED)) {
+
        client->refCount++;
        h_Hold_r(client->host);
        if (!client->deleted && client->prfail != 2) {