From 924fb5f5b96e4288a70bf64deee37886f78e2d38 Mon Sep 17 00:00:00 2001
From: Asanka Herath <asanka@mit.edu>
Date: Thu, 29 Jul 2004 14:53:35 +0000
Subject: [PATCH] server-ref-list-20040729

There was a race condition where between a thread calling cm_GetServerList and using the returned server list, another
thread could free the server list.  Now cm_GetServerList duplicates the server list with proper locks held.  Callers of
cm_GetServerList call cm_FreeServerList to free the returned list.
---
 src/WINNT/afsd/cm_conn.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/WINNT/afsd/cm_conn.c b/src/WINNT/afsd/cm_conn.c
index 856f31d822..4ac642db1d 100644
--- a/src/WINNT/afsd/cm_conn.c
+++ b/src/WINNT/afsd/cm_conn.c
@@ -103,6 +103,7 @@ long cm_GetServerList(struct cm_fid *fidp, struct cm_user *userp,
 	long code;
         cm_volume_t *volp = NULL;
         cm_serverRef_t *serversp = NULL;
+    cm_serverRef_t *newServersp = NULL;
         cm_cell_t *cellp = NULL;
 
         if (!fidp) {
@@ -116,6 +117,7 @@ long cm_GetServerList(struct cm_fid *fidp, struct cm_user *userp,
         code = cm_GetVolumeByID(cellp, fidp->volume, userp, reqp, &volp);
         if (code) return code;
         
+    lock_ObtainMutex(&volp->mx);
 	if (fidp->volume == volp->rwID)
         	serversp = volp->rwServersp;
 	else if (fidp->volume == volp->roID)
@@ -125,8 +127,36 @@ long cm_GetServerList(struct cm_fid *fidp, struct cm_user *userp,
 	else
 		serversp = NULL;
 
+    /* make a copy of the server list because by the time the 
+    caller tries to use it, it might have been freed.
+    Preserve server order. */
+    if(serversp) {
+        cm_serverRef_t ** nl;
+        cm_serverRef_t * tref;
+
+        nl = &newServersp;
+
+        lock_ObtainWrite(&cm_serverLock);
+        while(serversp) {
+
+            tref = malloc(sizeof(cm_serverRef_t));
+            tref->next = NULL;
+            tref->server = serversp->server;
+            tref->status = serversp->status;
+
+            tref->server->refCount++;
+
+            *nl = tref;
+            nl = &tref->next;
+
+            serversp = serversp->next;
+        }
+        lock_ReleaseWrite(&cm_serverLock);
+    }
+    lock_ReleaseMutex(&volp->mx);
+
         cm_PutVolume(volp);
-	*serverspp = serversp;
+    *serverspp = newServersp;
 	return 0;
 }
 
@@ -217,6 +247,7 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp,
 
             retry = 1;
         }
+        cm_FreeServerList(&serversp);
 
         if (fidp != NULL)   /* Not a VLDB call */
             cm_ForceUpdateVolume(fidp, userp, reqp);
@@ -231,6 +262,7 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp,
 				tsrp->status = not_busy;
 		}
         lock_ReleaseWrite(&cm_serverLock);
+        cm_FreeServerList(&serversp);
 		thrd_Sleep(5000);
 		retry = 1;
 	}
@@ -247,6 +279,7 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp,
 			}
 		}
         lock_ReleaseWrite(&cm_serverLock);
+        cm_FreeServerList(&serversp);
 		retry = 1;
 	}
 
@@ -279,11 +312,11 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp,
 
 		/* Mark server offline for this volume */
 		cm_GetServerList(fidp, userp, reqp, &serversp);
-
 		for (tsrp = serversp; tsrp; tsrp=tsrp->next) {
 			if (tsrp->server == serverp)
 				tsrp->status = offline;
 		}
+        cm_FreeServerList(&serversp);
 		retry = 1;
 	}
 
@@ -556,5 +589,6 @@ long cm_Conn(struct cm_fid *fidp, struct cm_user *userp, cm_req_t *reqp,
 	}
 
 	code = cm_ConnByMServers(serversp, userp, reqp, connpp);
+    cm_FreeServerList(&serversp);
         return code;
 }