Check for (hostFlags & HOSTDELETED) after h_Lock_r

Many callers of h_Lock_r do not check if the HOSTDELETED flag is set,
even though it could have been set while waiting for the host lock. Add
checks for it everywhere we call h_Lock_r and we care if the host has
been deleted.

FIXES 125507

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

Change-Id: I5eacee7a33f7b1357ec13cfcc4e1b7b4538837f6
Reviewed-on: http://gerrit.openafs.org/776
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 e8b5a40d1e94ed7c74c6414718d3b616d8ed511d..59abbad4b49afea2fdfb6e6f740e26159562848a 100644 (file)
--- a/src/viced/callback.c
+++ b/src/viced/callback.c
@@ -613,12 +613,13 @@ int
 AddCallBack1(struct host *host, AFSFid * fid, afs_uint32 * thead, int type,
             int locked)
 {
-    int retVal;
+    int retVal = 0;
     H_LOCK;
     if (!locked) {
        h_Lock_r(host);
     }
-    retVal = AddCallBack1_r(host, fid, thead, type, 1);
+    if (!(host->hostFlags & HOSTDELETED))
+        retVal = AddCallBack1_r(host, fid, thead, type, 1);
 
     if (!locked) {
        h_Unlock_r(host);
@@ -659,6 +660,11 @@ AddCallBack1_r(struct host *host, AFSFid * fid, afs_uint32 * thead, int type,
     if (!locked) {
        h_Lock_r(host);         /* this can yield, so do it before we get any */
        /* fragile info */
+        if (host->hostFlags & HOSTDELETED) {
+            host->Console &= ~2;
+            h_Unlock_r(host);
+            return 0;
+        }
     }
 
     fe = FindFE(fid);
@@ -839,12 +845,14 @@ MultiBreakCallBack_r(struct cbstruct cba[], int ncbas,
 
                        H_LOCK;
                        h_Lock_r(hp); 
-                       hp->hostFlags |= VENUSDOWN;
-               /**
-                 * We always go into AddCallBack1_r with the host locked
-                 */
-                       AddCallBack1_r(hp, afidp->AFSCBFids_val, itot(idx),
-                                      CB_DELAYED, 1);
+                        if (!(hp->hostFlags & HOSTDELETED)) {
+                            hp->hostFlags |= VENUSDOWN;
+                            /**
+                             * We always go into AddCallBack1_r with the host locked
+                             */
+                            AddCallBack1_r(hp, afidp->AFSCBFids_val, itot(idx),
+                                           CB_DELAYED, 1);
+                        }
                        h_Unlock_r(hp); 
                        H_UNLOCK;
                    }
@@ -979,6 +987,7 @@ DeleteCallBack(struct host *host, AFSFid * fid)
     cbstuff.DeleteCallBacks++;
 
     h_Lock_r(host);
+    /* do not care if the host has been HOSTDELETED */
     fe = FindFE(fid);
     if (!fe) {
        h_Unlock_r(host);
@@ -1748,7 +1757,7 @@ ClearHostCallbacks_r(struct host *hp, int locked)
     DeleteAllCallBacks_r(hp, 1);
     if (hp->hostFlags & VENUSDOWN) {
        hp->hostFlags &= ~RESETDONE;    /* remember that we must do a reset */
-    } else {
+    } else if (!(hp->hostFlags & HOSTDELETED)) {
        /* host is up, try a call */
        hp->hostFlags &= ~ALTADDR;      /* alternate addresses are invalid */
        cb_conn = hp->callback_rxcon;

diff --git a/src/viced/host.c b/src/viced/host.c
index 57fe1302b999acf107ea9cb2e9bdee575a4eb6b4..c481bee13a265cb0a77ef4454cf703e85ebc81c0 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -1410,13 +1410,13 @@ h_threadquota(int waiting)
 }
 
 /* Host is returned held */
+/* host should be released by caller if 0 == *heldp and non-null */
 struct host *
-h_GetHost_r(struct rx_connection *tcon)
+h_GetHost_r(struct rx_connection *tcon, int *heldp)
 {
     struct host *host;
     struct host *oldHost;
     int code;
-    int held;
     struct interfaceAddr interf;
     int interfValid = 0;
     struct Identity *identP = NULL;
@@ -1442,7 +1442,7 @@ h_GetHost_r(struct rx_connection *tcon)
     caps.Capabilities_len = 0;
 
     code = 0;
-    if (h_Lookup_r(haddr, hport, &held, &host))
+    if (h_Lookup_r(haddr, hport, heldp, &host))
        return 0;
     identP = (struct Identity *)rx_GetSpecific(tcon, rxcon_ident_key);
     if (host && !identP && !(host->Console & 1)) {
@@ -1452,20 +1452,23 @@ h_GetHost_r(struct rx_connection *tcon)
         */
        if ((host->hostFlags & HWHO_INPROGRESS) && 
            h_threadquota(host->lock.num_waiting)) {
-           if (!held)
+           if (!*heldp)
                h_Release_r(host);
            host = NULL;
            goto gethost_out;
        }
        h_Lock_r(host);
-       if (!(host->hostFlags & ALTADDR)) {
-           /* Another thread is doing initialization */
+       if (!(host->hostFlags & ALTADDR) ||
+            (host->hostFlags & HOSTDELETED)) {
+           /* Another thread is doing initialization
+             * or this host was deleted while we
+             * waited for the lock. */
            h_Unlock_r(host);
            ViceLog(125,
                    ("Host %s:%d starting h_Lookup again\n",
                     afs_inet_ntoa_r(host->host, hoststr),
                     ntohs(host->port)));
-           if (!held)
+           if (!*heldp)
                h_Release_r(host);
            goto retry;
        }
@@ -1537,7 +1540,7 @@ h_GetHost_r(struct rx_connection *tcon)
                    host->hostFlags &= ~HWHO_INPROGRESS;
                    host->hostFlags |= ALTADDR;
                    h_Unlock_r(host);
-                   if (!held)
+                   if (!*heldp)
                        h_Release_r(host);
                    host = NULL;
                    goto retry;
@@ -1553,7 +1556,7 @@ h_GetHost_r(struct rx_connection *tcon)
                 host->hostFlags &= ~HWHO_INPROGRESS;
                host->hostFlags |= ALTADDR;
                 h_Unlock_r(host);
-               if (!held)
+               if (!*heldp)
                    h_Release_r(host);
                host = NULL;
                goto retry;
@@ -1589,7 +1592,7 @@ h_GetHost_r(struct rx_connection *tcon)
                host->hostFlags &= ~HWHO_INPROGRESS;
                host->hostFlags |= ALTADDR;
                h_Unlock_r(host);
-               if (!held)
+               if (!*heldp)
                    h_Release_r(host);
                host = NULL;
                goto retry;
@@ -1659,7 +1662,7 @@ h_GetHost_r(struct rx_connection *tcon)
                host->hostFlags &= ~HWHO_INPROGRESS;
                host->hostFlags |= ALTADDR;
                h_Unlock_r(host);
-               if (!held)
+               if (!*heldp)
                    h_Release_r(host);
                host = NULL;
                rx_DestroyConnection(cb_in);
@@ -1694,7 +1697,7 @@ h_GetHost_r(struct rx_connection *tcon)
                    ("Host %s:%d starting h_Lookup again\n",
                     afs_inet_ntoa_r(host->host, hoststr),
                     ntohs(host->port)));
-           if (!held)
+           if (!*heldp)
                h_Release_r(host);
            goto retry;
        }
@@ -1723,12 +1726,13 @@ h_GetHost_r(struct rx_connection *tcon)
             h_Lock_r(host);
            host->hostFlags |= HOSTDELETED;
            h_Unlock_r(host);
-           if (!held)
+           if (!*heldp)
                h_Release_r(host);
            goto retry;
        }
     } else {
        host = h_Alloc_r(tcon); /* returned held and locked */
+       *heldp = 0;
        h_gethostcps_r(host, FT_ApproxTime());
        if (!(host->Console & 1)) {
            int pident = 0;
@@ -1793,13 +1797,27 @@ h_GetHost_r(struct rx_connection *tcon)
                cb_conn=NULL;
                H_LOCK;
            } else if (code == 0) {
+               int oldHeld = 1;
+
                oldHost = h_LookupUuid_r(&identP->uuid);
-                if (oldHost) {
-                    int probefail = 0;
 
-                   if (!h_Held_r(oldHost))
+                if (oldHost) {
+                   oldHeld = h_Held_r(oldHost);
+                   if (!oldHeld)
                        h_Hold_r(oldHost);
                    h_Lock_r(oldHost);
+
+                   if (oldHost->hostFlags & HOSTDELETED) {
+                       h_Unlock_r(oldHost);
+                       if (!oldHeld)
+                           h_Release_r(oldHost);
+                       oldHost = NULL;
+                   }
+               }
+
+               if (oldHost) {
+                   int probefail = 0;
+
                    oldHost->hostFlags |= HWHO_INPROGRESS;
 
                     if (oldHost->interface) {
@@ -1891,6 +1909,7 @@ h_GetHost_r(struct rx_connection *tcon)
                    /* release host because it was allocated by h_Alloc_r */
                    h_Release_r(host);
                    host = oldHost;
+                   *heldp = oldHeld;
                    /* the new host is held and locked */
                } else {
                    /* This really is a new host */
@@ -2116,6 +2135,7 @@ h_FindClient_r(struct rx_connection *tcon)
     char tcell[MAXKTCREALMLEN];
     int fail = 0;
     int created = 0;
+    int held = 1;
 
     client = (struct client *)rx_GetSpecific(tcon, rxcon_client_key);
     if (client && client->sid == rxr_CidOf(tcon) 
@@ -2187,10 +2207,10 @@ h_FindClient_r(struct rx_connection *tcon)
     }
 
     if (!client) { /* loop */
-       host = h_GetHost_r(tcon);       /* Returns it h_Held */
+       host = h_GetHost_r(tcon, &held);        /* Returns it h_Held */
 
        if (!host) 
-           return 0;
+           return NULL;
 
     retryfirstclient:
        /* First try to find the client structure */
@@ -2208,6 +2228,12 @@ h_FindClient_r(struct rx_connection *tcon)
        /* Still no client structure - get one */
        if (!client) {
            h_Lock_r(host);
+            if (host->hostFlags & HOSTDELETED) {
+                h_Unlock_r(host);
+               if (!held)
+                    h_Release_r(host);
+                return NULL;
+            }
            /* Retry to find the client structure */
            for (client = host->FirstClient; client; client = client->next) {
                if (!client->deleted && (client->sid == rxr_CidOf(tcon))
@@ -2329,6 +2355,26 @@ h_FindClient_r(struct rx_connection *tcon)
     /* Avoid chaining in more than once. */
     if (created) {
        h_Lock_r(host);
+
+        if (host->hostFlags & HOSTDELETED) {
+            h_Unlock_r(host);
+           if (!held)
+                h_Release_r(host);
+
+            host = NULL;
+            client->host = NULL;
+
+            if ((client->ViceId != ANONYMOUSID) && client->CPS.prlist_val)
+                free(client->CPS.prlist_val);
+            client->CPS.prlist_val = NULL;
+            client->CPS.prlist_len = 0;
+
+            client->refCount--;
+            ReleaseWriteLock(&client->lock);
+            FreeCE(client);
+            return NULL;
+        }
+
        client->next = host->FirstClient;
        host->FirstClient = client;
        h_Unlock_r(host);
@@ -2841,8 +2887,8 @@ CheckHost(register struct host *host, int held)
     }
     if (host->LastCall < checktime) {
        h_Lock_r(host);
-       host->hostFlags |= HWHO_INPROGRESS;
        if (!(host->hostFlags & HOSTDELETED)) {
+            host->hostFlags |= HWHO_INPROGRESS;
            cb_conn = host->callback_rxcon;
            rx_GetConnection(cb_conn);
            if (host->LastCall < clientdeletetime) {
@@ -2910,8 +2956,8 @@ CheckHost(register struct host *host, int held)
            rx_PutConnection(cb_conn);
            cb_conn=NULL;
            H_LOCK;
+            host->hostFlags &= ~HWHO_INPROGRESS;
        }
-       host->hostFlags &= ~HWHO_INPROGRESS;
        h_Unlock_r(host);
     }
     H_UNLOCK;

diff --git a/src/viced/host.h b/src/viced/host.h
index c333d40f05bd9588452c3a8b85525bfc1e9ca55b..473600e8a55fd912395c11892e6e1cad6eb75da1 100644 (file)
--- a/src/viced/host.h
+++ b/src/viced/host.h
@@ -211,7 +211,7 @@ extern void   hashInsert_r(afs_uint32 addr, afs_uint16 port,
                           struct host* host);
 extern struct host *h_LookupUuid_r(afsUUID * uuidp);
 extern void h_Enumerate(int (*proc) (), char *param);
-extern struct host *h_GetHost_r(struct rx_connection *tcon);
+extern struct host *h_GetHost_r(struct rx_connection *tcon, int *heldp);
 extern struct client *h_FindClient_r(struct rx_connection *tcon);
 extern int h_ReleaseClient_r(struct client *client);
 extern struct client *h_ID2Client(afs_int32 vid);