OpenAFS Master Repository branch, master, updated. openafs-devel-1_5_76-3114-gf74a0a7

Gerrit Code Review gerrit@openafs.org
Tue, 13 Nov 2012 10:38:09 -0800 (PST)


The following commit has been merged in the master branch:
commit 20b0c65a289e2b55fb6922c8f60e873f1f4c6f97
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Wed Oct 31 15:55:35 2012 -0500

    afs: Never use GetNewDSlot after init
    
    Currently there are two ways to get a dcache via a slot number:
    afs_GetNewDSot and afs_GetValidDSlot. afs_GetValidDSlot assumes that
    the given slot number refers to a dcache entry that is valid on disk;
    with afs_GetNewDSlot, the given slot may not be valid, and if it is
    not, an empty 'template' dcache is returned.
    
    afs_GetNewDSlot is useful for initializing cache files, since if a
    given dcache slot exists on disk and contains valid data, we use the
    dcache like normal. If it does not already exist or does not contain
    valid data, we fill in the missing data after afs_GetNewDSlot returns.
    
    However, for all other uses, afs_GetNewDSlot is incorrect, and causes
    various serious problems. After we have initialized our dcache
    entries, any attempt to read a dcache by slot number should succeed,
    since the number of dcache entries never changes after we are started,
    and we initialized all of them during client startup.
    
    Some code outside of afs_InitCacheFile was still using
    afs_GetNewDSlot; code that reads in a dslot from the free or discard
    list. In these cases, if there is any error reading the dcache slot
    from disk, we will be given a dcache that has some of its fields not
    filled in properly. Notably, we assume that the entry is not on the
    global hash table (we set tdc->f.fid.Fid.Volume to 0), and the
    tdc->f.inode field is not initialized at all, leaving it set to
    whatever was in memory for that tdc before we tried to read the slot
    from disk.
    
    This can cause cache corruption, since tdc->f.inode can point to the
    inoder for a different existing cache file, so writing to that dcache
    modifies the data for another cached file.
    
    To avoid this, modify the non-afs_InitCacheFile callers of
    afs_GetNewDSlot to avoid afs_GetNewDSlot. Since these callers read
    from the free/discard list, the contents of the dcache entries are not
    valid (the cell, volume, dv, etc are not valid), though they must
    exist on disk (we have a valid inode number for them). So, create a
    new function, afs_GetUnusedDSlot, to get a dcache that must exist on
    disk, but does not represent any valid data. Use this for callers that
    must get a dslot from the free/discard list.
    
    Add some comments to try and help explain what is going on.
    
    Change-Id: I5b1b7ef4886102a9e145320932b2fd650b5c6f2f
    Reviewed-on: http://gerrit.openafs.org/8370
    Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>

 src/afs/afs_chunkops.h   |   15 ++++++++++++---
 src/afs/afs_dcache.c     |   34 ++++++++++++++++++++++------------
 src/afs/afs_prototypes.h |    4 ++--
 3 files changed, 36 insertions(+), 17 deletions(-)

-- 
OpenAFS Master Repository