Windows: Avoid cm_Analyze race on cm_serverRef lists
authorJeffrey Altman <jaltman@your-file-system.com>
Mon, 18 Mar 2013 16:07:55 +0000 (12:07 -0400)
committerJeffrey Altman <jaltman@your-file-system.com>
Mon, 18 Mar 2013 21:26:24 +0000 (14:26 -0700)
commitb5675b57f815722f8c37fcfed5a2bd7b1ef112d6
tree0de58ffc9b58e629d1b0e5038b0f641e304839f9
parent19dc2fac408bd619f67c2e7ee29b42c81c1150bc
Windows: Avoid cm_Analyze race on cm_serverRef lists

cm_Analyze() accepted as a parameter a pointer to the first element
on a cm_serverRef list which is only ever used for VL operations.

cm_Analyze() would separately call cm_GetVolServerList() to obtain
the cm_serverRef list for RXAFS operations.  Then the variable 'serversp'
would be set to the first element of the list.

'serversp' was then used to refer to the list and would be passed to
cm_SetServerBusyStatus() and cm_ResetServerBusyStatus() which would
in turn obtain the cm_serverLock while it manipulated the cm_serverRef
status flags for the elements in the list.

The problem is that passing a pointer to the first element of the
cm_serverRef list without holding cm_serverLock can permit the list
contents to be altered including removal of the first element.  If the
race is lost and the memory associated with the first element is freed
before access, the afsd_service.exe will crash.

This patchset makes a number of changes.  First, the cm_serverRef_t
parameter is changed from a pointer to the first element of the list
to be a pointer to the HEAD pointer of the list.  Since it is ever only
used for cm_cell.vlServerp lists, the parameter is renamed to
'vlServerspp'.   Second, a separate "cm_serverRef_t ** volServerspp"
variable is allocated for the return value from the cm_GetVolServerList()
operations.

cm_SetServerBusyStatus() and cm_ResetServerBusyStatus() are altered to
accept a pointer to the HEAD of the list instead of a pointer to the first
element.  The cm_serverLock is now held read instead of write because the
list itself is not being altered.  All of the state changes being applied
to the cm_serverRef objects are atomic.

Finally, cm_serverLock is held across all list traversals within
cm_Analyze().  A read lock is obtained if the elements of the list are not
being removed or inserted and a write lock is obtained if they are.

Change-Id: I48464e90a828706dad442c019c75a717b06d690b
Reviewed-on: http://gerrit.openafs.org/9625
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>
src/WINNT/afsd/cm_conn.c
src/WINNT/afsd/cm_conn.h
src/WINNT/afsd/cm_volume.c