Correct the h_Enumerate_r hostList safety check

Ide1e5aca7c2c4a4af3f62bc07821db694f2f9999 added safety checks for a few
traversals through hostList, including the traversal in h_Enumerate_r.
Unfortunately, h_Enumerate_r may not hold H_LOCK over its entire
traversal (h_Release_r can drop and reacquire it), so the value of
hostCount is not guaranteed to stay the same.

A host may be deleted during the loop, or right near the end, decreasing
hostCount to below our current running 'count' of hosts, triggering the
panic unnecessarily. So instead, remember the value of hostCount.

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

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

diff --git a/src/viced/host.c b/src/viced/host.c
index 4c5dfab0e43b7b7a656073fd3465e61c2a26e3c9..efd7e5f6966080d5b839f9b59ea5cd03ebc0cda3 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -1100,6 +1100,10 @@ h_Enumerate(int (*proc) (), char *param)
  * host with respect to this lwp is passed to (*proc) as the param held.
  * The proc should return 0 if the host should be released, 1 if it should
  * be held after enumeration.
+ *
+ * @note Assumes that hostList is only prepended to, that a host is never
+ *       inserted into the middle. Otherwise this would not be guaranteed to
+ *       terminate.
  */
 void
 h_Enumerate_r(int (*proc) (), struct host *enumstart, char *param)
@@ -1108,13 +1112,18 @@ h_Enumerate_r(int (*proc) (), struct host *enumstart, char *param)
     int held = 0;
     int nheld = 0;
     int count;
+    int origHostCount;
 
     if (hostCount == 0) {
        return;
     }
     if (enumstart && !(held = h_Held_r(enumstart)))
        h_Hold_r(enumstart); 
-    for (count = 0, host = enumstart; host && count < hostCount; host = next, held = nheld, count++) {
+    /* 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;
        if (next && !(nheld = h_Held_r(next)))
            h_Hold_r(next);
@@ -1122,8 +1131,8 @@ h_Enumerate_r(int (*proc) (), struct host *enumstart, char *param)
        if (!held)
            h_Release_r(host); /* this might free up the host */
     }
-    if (host != NULL) {
-       ViceLog(0, ("h_Enumerate_r found more than %d hosts\n", hostCount));
+    if (host != NULL && count >= origHostCount) {
+       ViceLog(0, ("h_Enumerate_r found more than %d hosts\n", origHostCount));
        ShutDownAndCore(PANIC);
     }
 }                              /*h_Enumerate_r */