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>
This commit is contained in:
Andrew Deason 2010-02-12 17:44:31 -06:00 committed by Derrick Brashear
parent 476debff7f
commit 70798bd662
2 changed files with 80 additions and 18 deletions

View File

@ -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);

View File

@ -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) {