Race conditions in getnetconfig

The clnt_* functions are *not* thread safe. Race conditions are caused
by the functions setnetconfig , getnetconfig, endnetconfig and
getnetconfigent that accesses global static data nc_file and ni which
are defined in the file getnetconfig are *not* protected by any mutex.
When more than one thread access them the variables become a nonlocal
side effect . These race conditions causing process to give undesired
behavior and leading to crash on file operations mostly on fclose. By
introducing the mutex nc_db_lock the netconfig database is synchronized
and prevented from crash.

Signed-off-by: Susant Sahani <ssahani@redhat.com>
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/src/getnetconfig.c b/src/getnetconfig.c
index 2460a6e0d6f4729044cbd556000ccdc3f1ee519b..78de0f6bd523ded16bdf30729de9b192ebb2cb43 100644 (file)
--- a/src/getnetconfig.c
+++ b/src/getnetconfig.c
@@ -120,6 +120,7 @@ static struct netconfig *dup_ncp(struct netconfig *);
 
 static FILE *nc_file;          /* for netconfig db */
 static struct netconfig_info   ni = { 0, 0, NULL, NULL};
+extern pthread_mutex_t nc_db_lock;
 
 #define MAXNETCONFIGLINE    1000
 
@@ -192,14 +193,17 @@ setnetconfig()
      * For multiple calls, i.e. nc_file is not NULL, we just return the
      * handle without reopening the netconfig db.
      */
+    mutex_lock(&nc_db_lock);
     ni.ref++;
     if ((nc_file != NULL) || (nc_file = fopen(NETCONFIG, "r")) != NULL) {
        nc_vars->valid = NC_VALID;
        nc_vars->flag = 0;
        nc_vars->nc_configs = ni.head;
+       mutex_unlock(&nc_db_lock);
        return ((void *)nc_vars);
     }
     ni.ref--;
+    mutex_unlock(&nc_db_lock);
     nc_error = NC_NONETCONFIG;
     free(nc_vars);
     return (NULL);
@@ -222,12 +226,15 @@ void *handlep;
     char *stringp;             /* tmp string pointer */
     struct netconfig_list      *list;
     struct netconfig *np;
+    struct netconfig *result;
 
     /*
      * Verify that handle is valid
      */
+    mutex_lock(&nc_db_lock);
     if (ncp == NULL || nc_file == NULL) {
        nc_error = NC_NOTINIT;
+       mutex_unlock(&nc_db_lock);
        return (NULL);
     }
 
@@ -244,11 +251,14 @@ void *handlep;
        if (ncp->flag == 0) {   /* first time */
            ncp->flag = 1;
            ncp->nc_configs = ni.head;
-           if (ncp->nc_configs != NULL)        /* entry already exist */
+           if (ncp->nc_configs != NULL)        /* entry already exist */ {
+               mutex_unlock(&nc_db_lock);
                return(ncp->nc_configs->ncp);
+               }
        }
        else if (ncp->nc_configs != NULL && ncp->nc_configs->next != NULL) {
            ncp->nc_configs = ncp->nc_configs->next;
+           mutex_unlock(&nc_db_lock);
            return(ncp->nc_configs->ncp);
        }
 
@@ -256,16 +266,22 @@ void *handlep;
         * If we cannot find the entry in the list and is end of file,
         * we give up.
         */
-       if (ni.eof == 1)        return(NULL);
+       if (ni.eof == 1) {
+           mutex_unlock(&nc_db_lock);
+           return(NULL);
+       }
        break;
     default:
        nc_error = NC_NOTINIT;
+       mutex_unlock(&nc_db_lock);
        return (NULL);
     }
 
     stringp = (char *) malloc(MAXNETCONFIGLINE);
-    if (stringp == NULL)
-       return (NULL);
+    if (stringp == NULL) {
+    mutex_unlock(&nc_db_lock);
+    return (NULL);
+    }
 
 #ifdef MEM_CHK
     if (malloc_verify() == 0) {
@@ -281,6 +297,7 @@ void *handlep;
        if (fgets(stringp, MAXNETCONFIGLINE, nc_file) == NULL) {
            free(stringp);
            ni.eof = 1;
+           mutex_unlock(&nc_db_lock);
            return (NULL);
         }
     } while (*stringp == '#');
@@ -288,12 +305,14 @@ void *handlep;
     list = (struct netconfig_list *) malloc(sizeof (struct netconfig_list));
     if (list == NULL) {
        free(stringp);
+               mutex_unlock(&nc_db_lock);
        return(NULL);
     }
     np = (struct netconfig *) malloc(sizeof (struct netconfig));
     if (np == NULL) {
        free(stringp);
-       free(list);
+               free(list);
+               mutex_unlock(&nc_db_lock);
        return(NULL);
     }
     list->ncp = np;
@@ -304,6 +323,7 @@ void *handlep;
        free(stringp);
        free(np);
        free(list);
+       mutex_unlock(&nc_db_lock);
        return (NULL);
     }
     else {
@@ -321,7 +341,9 @@ void *handlep;
            ni.tail = ni.tail->next;
        }
        ncp->nc_configs = ni.tail;
-       return(ni.tail->ncp);
+       result = ni.tail->ncp;
+       mutex_unlock(&nc_db_lock);
+       return result;
     }
 }
 
@@ -355,8 +377,10 @@ void *handlep;
     nc_handlep->valid = NC_INVALID;
     nc_handlep->flag = 0;
     nc_handlep->nc_configs = NULL;
+    mutex_lock(&nc_db_lock);
     if (--ni.ref > 0) {
-       free(nc_handlep);
+       mutex_unlock(&nc_db_lock);
+       free(nc_handlep);
        return(0);
     }
 
@@ -377,9 +401,11 @@ void *handlep;
        q = p;
     }
     free(nc_handlep);
-
-    fclose(nc_file);
+    if(nc_file != NULL) {
+        fclose(nc_file);
+    }
     nc_file = NULL;
+    mutex_unlock(&nc_db_lock);
     return (0);
 }
 
@@ -427,16 +453,21 @@ getnetconfigent(netid)
      * If all the netconfig db has been read and placed into the list and
      * there is no match for the netid, return NULL.
      */
+    mutex_lock(&nc_db_lock);
     if (ni.head != NULL) {
        for (list = ni.head; list; list = list->next) {
            if (strcmp(list->ncp->nc_netid, netid) == 0) {
-               return(dup_ncp(list->ncp));
+                       ncp = dup_ncp(list->ncp);
+                       mutex_unlock(&nc_db_lock);
+                       return ncp;
            }
        }
-       if (ni.eof == 1)        /* that's all the entries */
-               return(NULL);
+        if (ni.eof == 1) {     /* that's all the entries */
+               mutex_unlock(&nc_db_lock);
+               return(NULL);
+        }
     }
-
+    mutex_unlock(&nc_db_lock);
 
     if ((file = fopen(NETCONFIG, "r")) == NULL) {
        nc_error = NC_NONETCONFIG;

diff --git a/src/mt_misc.c b/src/mt_misc.c
index fe12c31e23e81cd4fcb40b3767bdc2f67449480f..b24c130babca466e725326cebaa90309134b3dde 100644 (file)
--- a/src/mt_misc.c
+++ b/src/mt_misc.c
@@ -91,6 +91,9 @@ pthread_mutex_t       xprtlist_lock = PTHREAD_MUTEX_INITIALIZER;
 /* serializes calls to public key routines */
 pthread_mutex_t serialize_pkey = PTHREAD_MUTEX_INITIALIZER;
 
+/* protects global variables ni and nc_file (getnetconfig.c) */
+pthread_mutex_t nc_db_lock = PTHREAD_MUTEX_INITIALIZER;
+
 #undef rpc_createerr
 
 struct rpc_createerr rpc_createerr;