h_TossStuff_r: make sure host does not go away

When h_TossStuff_r h_NBLock_r's a host, it is not only possible for
someone else to grab a hold on the host, but in theory it's also
possible for someone to hold a host, release it, and for the host to be
deleted again (assuming some callers hold HOSTDELETED hosts, which they
should not be doing).

To make this safety check a bit more robust, hold the host in
h_TossStuff_r before h_NBLock_r'ing, to ensure that it does not go away
while we're waiting for H_LOCK.

Reviewed-on: http://gerrit.openafs.org/1330
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit 380bd77c328d8d83a007cd97f6564fcb5b990a73)

Change-Id: I19d001dba8b0c96b694aefdba2475f0103d33e44
Reviewed-on: http://gerrit.openafs.org/1368
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit b78eeb0cca48c65d4466052d207fa22f08a8fa70)

diff --git a/src/viced/host.c b/src/viced/host.c
index 3711645e173313b44b490a6d6a3cf1c9b59dd34e..8f00094040a760dd924399d234d18a3a7e2db361 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -503,15 +503,17 @@ h_Release_r(register struct host *host)
        if (!h_OtherHolds_r(host)) {
            /* must avoid masking this until after h_OtherHolds_r runs
             * but it should be run before h_TossStuff_r */
-           (host)->holds[h_holdSlot()] &= ~h_holdbit();
+           h_Decrement_r(host);
            if ((host->hostFlags & HOSTDELETED)
                || (host->hostFlags & CLIENTDELETED)) {
                h_TossStuff_r(host);
            }
-       } else
-           (host)->holds[h_holdSlot()] &= ~h_holdbit();
-    } else
-       (host)->holds[h_holdSlot()] &= ~h_holdbit();
+       } else {
+           h_Decrement_r(host);
+       }
+    } else {
+       h_Decrement_r(host);
+    }
 
     return 0;
 }
@@ -913,9 +915,18 @@ h_TossStuff_r(register struct host *host)
 {
     register struct client **cp, *client;
     int i;
+    int code;
+
+    /* make sure host doesn't go away over h_NBLock_r */
+    h_Hold_r(host);
+
+    code = h_NBLock_r(host);
+
+    /* don't use h_Release_r, since that may call h_TossStuff_r again */
+    h_Decrement_r(host);
 
     /* if somebody still has this host locked */
-    if (h_NBLock_r(host) != 0) {
+    if (code != 0) {
        char hoststr[16];
        ViceLog(0,
                ("Warning:  h_TossStuff_r failed; Host %s:%d was locked.\n",

diff --git a/src/viced/host.h b/src/viced/host.h
index 473600e8a55fd912395c11892e6e1cad6eb75da1..d07fc3c266dea5b39ad249599f52972ec5f86e35 100644 (file)
--- a/src/viced/host.h
+++ b/src/viced/host.h
@@ -176,6 +176,7 @@ extern char *h_UserName(struct client *client);
 #define h_holdbit()  ( 1<<h_holdIndex() )
 
 #define h_Hold_r(host)   ((host)->holds[h_holdSlot()] |= h_holdbit())
+#define h_Decrement_r(host) ((host)->holds[h_holdSlot()] &= ~h_holdbit())
 extern int h_Release(register struct host *host);
 extern int h_Release_r(register struct host *host);