OpenAFS Master Repository branch, openafs-devel-1_7_x, updated. openafs-devel-1_7_22-11-g815ec40

Gerrit Code Review gerrit@openafs.org
Mon, 18 Mar 2013 16:27:47 -0700 (PDT)


The following commit has been merged in the openafs-devel-1_7_x branch:
commit 815ec40ba0b99124b0e690662fa884dc96fc4ece
Author: Jeffrey Altman <jaltman@your-file-system.com>
Date:   Mon Mar 18 12:07:55 2013 -0400

    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.
    
    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>
    (cherry picked from commit b5675b57f815722f8c37fcfed5a2bd7b1ef112d6)
    
    Change-Id: I24b318948f876dacba155752900889eb63b5b69b
    Reviewed-on: http://gerrit.openafs.org/9629
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>

 src/WINNT/afsd/cm_conn.c   |  200 ++++++++++++++++++++++----------------------
 src/WINNT/afsd/cm_conn.h   |    2 +-
 src/WINNT/afsd/cm_volume.c |    4 +-
 3 files changed, 102 insertions(+), 104 deletions(-)

-- 
OpenAFS Master Repository