From: Andrew Deason Date: Fri, 12 Feb 2010 23:44:31 +0000 (-0600) Subject: Check for HOSTDELETED before h_Hold_r X-Git-Tag: openafs-devel-1_5_73~150 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=b8c3c6add90ea3face9a16ff04a1024be3d8f32d;p=packages%2Fo%2Fopenafs.git 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 Change-Id: I5a61831f5afdbc908b82e4cf63cf14a34a36e275 Reviewed-on: http://gerrit.openafs.org/1305 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- diff --git a/src/viced/callback.c b/src/viced/callback.c index cbe90d770..117a48e41 100644 --- a/src/viced/callback.c +++ b/src/viced/callback.c @@ -849,10 +849,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 @@ -1232,11 +1234,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); @@ -1383,9 +1388,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); @@ -1624,6 +1631,13 @@ ClearHostCallbacks_r(struct host *hp, int locked) ("GSS: Delete longest inactive host %p (%s:%d)\n", hp, afs_inet_ntoa_r(hp->host, hoststr), ntohs(hp->port))); + 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; + } + h_Hold_r(hp); /** Try a non-blocking lock. If the lock is already held return diff --git a/src/viced/host.c b/src/viced/host.c index 6e7476871..2acce8c4a 100644 --- a/src/viced/host.c +++ b/src/viced/host.c @@ -972,6 +972,7 @@ h_Enumerate(int (*proc) (struct host*, int, void *), void *param) register struct host *host, **list; register int *flags; register int i, count; + int totalCount; H_LOCK; if (hostCount == 0) { @@ -988,12 +989,18 @@ h_Enumerate(int (*proc) (struct host*, int, void *), void *param) ViceLog(0, ("Failed malloc in h_Enumerate (flags)\n")); assert(0); } - for (count = 0, host = hostList; host && count < hostCount; host = host->next, count++) { - list[count] = 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; + 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); @@ -1042,6 +1049,31 @@ h_Enumerate_r(int (*proc) (struct host *, int, void *), 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; + } + h_Hold_r(enumstart); /* remember hostCount, lest it change over the potential H_LOCK drop in @@ -1050,12 +1082,25 @@ h_Enumerate_r(int (*proc) (struct host *, int, void *), for (count = 0, host = enumstart; host && count < origHostCount; host = next, flags = nflags, 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 && !H_ENUMERATE_ISSET_BAIL(flags)) h_Hold_r(next); - flags = (*proc) (host, flags, param); - if (H_ENUMERATE_ISSET_BAIL(flags)) { - h_Release_r(host); /* this might free up the host */ - break; + + if (!(host->hostFlags & HOSTDELETED)) { + flags = (*proc) (host, flags, param); + if (H_ENUMERATE_ISSET_BAIL(flags)) { + h_Release_r(host); /* this might free up the host */ + break; + } } h_Release_r(host); /* this might free up the host */ } @@ -2185,7 +2230,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) {