viced: Keep H_LOCK while locking host in h_Alloc_r
authorAndrew Deason <adeason@sinenomine.net>
Tue, 20 Dec 2011 22:44:42 +0000 (17:44 -0500)
committerDerrick Brashear <shadow@dementix.org>
Wed, 8 Feb 2012 21:51:34 +0000 (13:51 -0800)
Currently in h_Alloc_r, we h_Lock_r the host, so we have it locked on
return. However, h_Lock_r drops the host glock, which is bad in this
situation since we have already added the host to the global hash
table, so other threads may see it. This can mean that by the time
h_Alloc_r returns, the returned host may have HOSTDELETED set, and/or
the addresses associated with the host may be completely different.

h_Alloc_r's caller, h_GetHost_r, seems to assume that the host is
still associated with the address of the passed-in connection. When
this is not true, this can result in the host structure getting into a
strange state, such as the primary addr/port may not be hashed. The
host may also have HOSTDELETED set, in which case we're not supposed
to be dealing with it at all.

To avoid these problems, lock host->lock directly in h_Alloc_r,
without going through h_Lock_r and dropping H_LOCK. Also do it as one
of the first things we do to initialize the host, just to make sure
that if anybody else happens to see the host, it is locked by us when
they do.

Change-Id: Ia99cb84ad94f3e143ed0bae33485a88d60ff5b27
Reviewed-on: http://gerrit.openafs.org/6389
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>

src/viced/host.c

index 4922c83..08632b0 100644 (file)
@@ -654,6 +654,12 @@ h_Alloc_r(struct rx_connection *r_con)
     if (!host)
        return NULL;
 
+    h_Hold_r(host);
+    /* acquire the host lock withot dropping H_LOCK. we can do this here
+     * because we know we will not block; we just created this host and
+     * nobody else knows about it. */
+    ObtainWriteLock(&host->lock);
+
     host->host = rxr_HostOf(r_con);
     host->port = rxr_PortOf(r_con);
 
@@ -685,8 +691,6 @@ h_Alloc_r(struct rx_connection *r_con)
     h_gethostcps(host);                /* do this under host hold/lock */
 #endif
     host->FirstClient = NULL;
-    h_Hold_r(host);
-    h_Lock_r(host);
     h_InsertList_r(host);      /* update global host List */
     /*
      * Compare the new host's IP address (in host byte order) with ours