bucoord: Remove theoretical overflow of ubik array
authorSimon Wilkinson <sxw@your-file-system.com>
Fri, 1 Mar 2013 11:09:04 +0000 (11:09 +0000)
committerDerrick Brashear <shadow@your-file-system.com>
Mon, 4 Mar 2013 03:01:05 +0000 (19:01 -0800)
The ubik connections array is NULL terminated, so we have to
ensure that there is enough space for the trailing NULL. As the
array is MAXSERVERS elements long, this means that we can only
store MAXSERVERS-1 connections in it.

This problem will never be encountered by the correct code, as
the number of hosts returned from afsconf_Open is capped at
MAXHOSTSPERCELL (currently 8). MAXSERVERS is currently 20. However,
fix the bug in case we increase MAXHOSTSPERCELL at some point in
the future.

Caught by coverity (#985576)

Change-Id: Icd3f4afe929cbf05522e44132f055a3955c4d23c
Reviewed-on: http://gerrit.openafs.org/9322
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>

src/bucoord/ubik_db_if.c

index fae1286..991e0a1 100644 (file)
@@ -902,11 +902,15 @@ udbClientInit(int noAuthFlag, int localauth, char *cellName)
     if (&udbHandle.uh_scIndex == RX_SECIDX_NULL && !noAuthFlag)
        afs_com_err(whoami, 0, "Can't get tokens - running unauthenticated");
 
-    if (info.numServers > MAXSERVERS) {
+    /* We have to have space for the trailing NULL that terminates the server
+     * conneciton array - so we can only store MAXSERVERS-1 real elements in
+     * that array.
+     */
+    if (info.numServers >= MAXSERVERS) {
        afs_com_err(whoami, 0,
                "Warning: %d BDB servers exist for cell '%s', can only remember the first %d",
-               info.numServers, cellName, MAXSERVERS);
-       info.numServers = MAXSERVERS;
+               info.numServers, cellName, MAXSERVERS-1);
+       info.numServers = MAXSERVERS - 1;
     }
 
     /* establish connections to the servers. Check for failed connections? */