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.

Also, we break out of the loop if the H_ENUMERATE_ISSET_BAIL test
succeeds, making the panic also incorrectly trigger then.

So instead, remember the value of hostCount, and ensure that we've
actually exceeded that count in the post-loop check.

Change-Id: I7c13bbf111b592df6860e5852807fa463c3ebd7e
Reviewed-on: http://gerrit.openafs.org/1304
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/viced/host.c b/src/viced/host.c
index 46d4ba93aef0b9768329d7a440b32d3acf3de536..6e747687194005c3c1b74f81736d184052a07fcd 100644 (file)
--- a/src/viced/host.c
+++ b/src/viced/host.c
@@ -1021,6 +1021,10 @@ h_Enumerate(int (*proc) (struct host*, int, void *), void *param)
  * Needed?  Why not always h_Hold_r and h_Release_r in (*proc), or even -never-
  * h_Hold_r or h_Release_r in (*proc)?
  *
+ * @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.
+ *
  * **The proc should return 0 if the host should be released, 1 if it should
  * be held after enumeration.
  */
@@ -1032,12 +1036,19 @@ h_Enumerate_r(int (*proc) (struct host *, int, void *),
     int flags = 0;
     int nflags = 0;
     int count;
+    int origHostCount;
 
     if (hostCount == 0) {
        return;
     }
+
     h_Hold_r(enumstart);
-    for (count = 0, host = enumstart; host && count < hostCount; host = next, flags = nflags, 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, flags = nflags, count++) {
        next = host->next;
        if (next && !H_ENUMERATE_ISSET_BAIL(flags))
            h_Hold_r(next);
@@ -1048,8 +1059,8 @@ h_Enumerate_r(int (*proc) (struct host *, int, void *),
        }
        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 */