auth: fix afsconf_GetExtendedCellInfo memory leak 96/13396/7
authorMichael Meffie <mmeffie@sinenomine.net>
Thu, 15 Nov 2018 21:19:51 +0000 (16:19 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 25 Jan 2019 13:45:19 +0000 (08:45 -0500)
Commit c4a127d0578e521b97131c5dedf9da58f71b0242
(ubik-clone-support-20010212) added changes to support ubik clone sites.
This commit added the afsconf_GetExtendedCellInfo function, which
returns the info given by the original afsconf_GetCellInfo, plus an
array of booleans (as chars) to indicate which cell servers are ubik
clones.

Unfortunately, the afsconf_GetExtendedCellInfo function calls the
afsconf_OpenInternal function on an already opened configuration. It
does so to look for server entries which are marked as clone sites in
the CellServDB file. Opening the already opened configuration leaks at
least the cellName and local realms information, and is generally
confusing.

Instead, remember which sites are designated as clone sites when the
CellServDB is read when the configuration is opened, and return that
info to the callers of afsconf_GetExtendedCellInfo.

This commit adds the clone array to the afsconf_cell structure and
changes to afsconf_GetCellInfo() for this new server-related data.

As part of this change, remove the no longer needed cell and clones
arguments to the internal function afsconf_OpenInternal, which were
added by commit c4a127d0578e521b97131c5dedf9da58f71b0242.

Update the testcellconfig test program to output the new afsconf_cell
clone member.

This leak was found with valgrind.

Change-Id: I73db60b6a4a77e620e0511ca45cc3418503278a4
Reviewed-on: https://gerrit.openafs.org/13396
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

src/auth/cellconfig.c
src/auth/cellconfig.p.h
src/auth/test/testcellconf.c

index 5861874..5ec97bf 100644 (file)
@@ -73,10 +73,9 @@ static int TrimLine(char *abuffer, int abufsize);
 static int GetCellNT(struct afsconf_dir *adir);
 #endif
 static int GetCellUnix(struct afsconf_dir *adir);
-static int afsconf_OpenInternal(struct afsconf_dir *adir, char *cell,
-                               char clones[]);
+static int afsconf_OpenInternal(struct afsconf_dir *adir);
 static int ParseHostLine(char *aline, struct sockaddr_in *addr,
-                        char *aname, char *aclone);
+                        char *aname, char *aclone /* boolean */);
 static int ParseCellLine(char *aline, char *aname,
                         char *alname);
 static int afsconf_CloseInternal(struct afsconf_dir *adir);
@@ -433,7 +432,7 @@ afsconf_Open(const char *adir)
     tdir = calloc(1, sizeof(struct afsconf_dir));
     tdir->name = strdup(adir);
 
-    code = afsconf_OpenInternal(tdir, 0, 0);
+    code = afsconf_OpenInternal(tdir);
     if (code) {
        char *afsconf_path, afs_confdir[128];
 
@@ -481,7 +480,7 @@ afsconf_Open(const char *adir)
            afsconf_path = afs_confdir;
        }
        tdir->name = strdup(afsconf_path);
-       code = afsconf_OpenInternal(tdir, 0, 0);
+       code = afsconf_OpenInternal(tdir);
        if (code) {
            free(tdir->name);
            goto fail;
@@ -609,8 +608,7 @@ cm_enumCellRegistryProc(void *rockp, char * cellNamep)
 
 
 static int
-afsconf_OpenInternal(struct afsconf_dir *adir, char *cell,
-                    char clones[])
+afsconf_OpenInternal(struct afsconf_dir *adir)
 {
     afsconf_FILE *tf;
     char *tp, *bp;
@@ -711,18 +709,10 @@ afsconf_OpenInternal(struct afsconf_dir *adir, char *cell,
            }
            i = curEntry->cellInfo.numServers;
            if (i < MAXHOSTSPERCELL) {
-               if (cell && !strcmp(cell, curEntry->cellInfo.name))
-                   code =
-                       ParseHostLine(tbuffer,
-                                     &curEntry->cellInfo.hostAddr[i],
-                                     curEntry->cellInfo.hostName[i],
-                                     &clones[i]);
-               else
-                   code =
-                       ParseHostLine(tbuffer,
-                                     &curEntry->cellInfo.hostAddr[i],
-                                     curEntry->cellInfo.hostName[i], 0);
-
+               code = ParseHostLine(tbuffer,
+                                    &curEntry->cellInfo.hostAddr[i],
+                                    curEntry->cellInfo.hostName[i],
+                                    &curEntry->cellInfo.clone[i]);
                if (code) {
                    if (code == AFSCONF_SYNTAX) {
                        for (bp = tbuffer; *bp != '\n'; bp++) { /* Take out the <cr> from the buffer */
@@ -829,7 +819,7 @@ afsconf_OpenInternal(struct afsconf_dir *adir, char *cell,
  */
 static int
 ParseHostLine(char *aline, struct sockaddr_in *addr, char *aname,
-             char *aclone)
+             char *aclone /* boolean */)
 {
     int i;
     int c[4];
@@ -943,18 +933,15 @@ afsconf_GetExtendedCellInfo(struct afsconf_dir *adir, char *acellName,
                            char clones[])
 {
     afs_int32 code;
-    char *cell;
+    int i;
 
     code = afsconf_GetCellInfo(adir, acellName, aservice, acellInfo);
     if (code)
        return code;
 
-    if (acellName)
-       cell = acellName;
-    else
-       cell = (char *)&acellInfo->name;
-
-    code = afsconf_OpenInternal(adir, cell, clones);
+    for (i = 0; i < acellInfo->numServers; i++) {
+       clones[i] = acellInfo->clone[i];
+    }
     return code;
 }
 
@@ -1404,9 +1391,11 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
             short numServers=0;                                        /*Num active servers for the cell */
             struct sockaddr_in hostAddr[MAXHOSTSPERCELL];      /*IP addresses for cell's servers */
             char hostName[MAXHOSTSPERCELL][MAXHOSTCHARS];      /*Names for cell's servers */
+            char clone[MAXHOSTSPERCELL];                       /*Indicates which ones are clones. */
 
             memset(&hostAddr, 0, sizeof(hostAddr));
             memset(&hostName, 0, sizeof(hostName));
+            memset(&clone, 0, sizeof(clone));
 
             for ( j=0; j<acellInfo->numServers && numServers < MAXHOSTSPERCELL; j++ ) {
                 struct hostent *he = gethostbyname(acellInfo->hostName[j]);
@@ -1435,6 +1424,7 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
 #endif
                         memcpy(&hostAddr[numServers].sin_addr.s_addr, he->h_addr_list[i], sizeof(afs_uint32));
                         strcpy(hostName[numServers], acellInfo->hostName[j]);
+                        clone[numServers] = acellInfo->clone[j];
                         foundAddr = 1;
                         numServers++;
                     }
@@ -1442,6 +1432,7 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
                 if (!foundAddr) {
                     hostAddr[numServers] = acellInfo->hostAddr[j];
                     strcpy(hostName[numServers], acellInfo->hostName[j]);
+                    clone[numServers] = acellInfo->clone[j];
                     numServers++;
                 }
             }
@@ -1449,6 +1440,7 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
             for (i=0; i<numServers; i++) {
                 acellInfo->hostAddr[i] = hostAddr[i];
                 strcpy(acellInfo->hostName[i], hostName[i]);
+                acellInfo->clone[i] = clone[i];
             }
             acellInfo->numServers = numServers;
             acellInfo->flags |= AFSCONF_CELL_FLAG_DNS_QUERIED;
@@ -1588,6 +1580,6 @@ afsconf_Reopen(struct afsconf_dir *adir)
     code = afsconf_CloseInternal(adir);
     if (code)
        return code;
-    code = afsconf_OpenInternal(adir, 0, 0);
+    code = afsconf_OpenInternal(adir);
     return code;
 }
index 5a8da04..c9423c0 100644 (file)
@@ -58,6 +58,7 @@ struct afsconf_cell {
     short flags;               /* useful flags */
     struct sockaddr_in hostAddr[MAXHOSTSPERCELL];      /*IP addresses for cell's servers */
     char hostName[MAXHOSTSPERCELL][MAXHOSTCHARS];      /*Names for cell's servers */
+    char clone[MAXHOSTSPERCELL];                       /*Indicates which ones are clones */
     char *linkedCell;          /* Linked cell name, if any */
     int timeout;               /* Data timeout, if non-zero */
 };
index 022e262..a16a9dd 100644 (file)
@@ -57,8 +57,8 @@ PrintOneCell(struct afsconf_cell *ainfo, void *arock, struct afsconf_dir *adir)
     printf("Cell %s:\n", ainfo->name);
     for (i = 0; i < ainfo->numServers; i++) {
        memcpy(&temp, &ainfo->hostAddr[i].sin_addr, sizeof(long));
-       printf(" %d host %s at %lx port %x\n", i, ainfo->hostName[i], temp,
-              ainfo->hostAddr[i].sin_port);
+       printf(" %d host %s at %lx port %x clone %s\n", i, ainfo->hostName[i], temp,
+              ainfo->hostAddr[i].sin_port, (ainfo->clone[i] ? "yes" : "no"));
     }
     return 0;
 }