OpenAFS Master Repository branch, master, updated. BP-openafs-stable-1_8_x-565-g7e41ee0

[Gerrit Code Review]

The following commit has been merged in the master branch:
commit 7e41ee0bd50d39a356f0435ff370a0a7be40306f
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Tue Apr 7 13:15:31 2020 -0500

    vlserver: Correctly pad nvlentry for "O" RPCs
    
    For our old-style "O" RPCs (e.g. VL_CreateEntry, instead of
    VL_CreateEntryN), vlserver calls vldbentry_to_vlentry to convert to
    the internal 'struct nvlentry' format. After all of the sites have
    been copied to the internal format, we fill the remaining sites by
    setting the serverNumber to BADSERVERID. For nvldbentry_to_vlentry, we
    do this for NMAXNSERVERS sites, but for vldbentry_to_vlentry, we do
    this for OMAXNSERVERS.
    
    The thing is, both functions are filling in entries for a 'struct
    nvlentry', which has NMAXNSERVERS 'serverNumber' entries. So for
    vldbentry_to_vlentry, we are skipping setting the last few sites
    (specifically, NMAXNSERVERS-OMAXNSERVERS = 13-8 = 5).
    
    This can easily cause our O-style RPCs to write out entries to disk
    that have uninitialized sites at the end of the array. For example, an
    entry with one site should have server numbers that look like this:
    
        serverNumber = {1, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255}
    
    That is, one real serverid (a '1' here), followed by twelve
    BADSERVERIDs.
    
    But for a VL_CreateEntry call, the 'struct nvlentry' is zeroed out
    before vldbentry_to_vlentry is called, and so the server numbers in
    the written entry look like this:
    
        serverNumber = {1, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 0}
    
    That is, one real serverid (a '1' here), followed by seven
    BADSERVERIDs, followed by five '0's.
    
    Most of the time, this is not noticeable, since our code that reads in
    entries from disk stops processing sites when we encounter the first
    BADSERVERID site (see vlentry_to_nvldbentry). However, if the entry
    has 8 sites, then none of the entries will contain BADSERVERID, and so
    we will actually process the trailing 5 bogus sites. This would appear
    as 5 extra volume sites for a volume, most likely all for the same
    server.
    
    For VL_CreateEntry, the vlentry struct is always zeroed before we use
    it, so the trailing sites will always be filled with 0. For
    VL_ReplaceEntry, the trailing sites will be unchanged from whatever
    was read in from the existing disk entry.
    
    To fix this, just change the relevant loop to go through NMAXNSERVERS
    entries, so we actually go to the end of the serverNumber (et al)
    array.
    
    This may appear similar to commit ddf7d2a7 (vlserver: initialize
    nvlentry elements after read). However, that commit fixed a case
    involving the old vldb database format (which hopefully is not being
    used). This commit fixes a case where we are using the new vldb
    database format, but with the old RPCs, which may still be used by old
    tools.
    
    Change-Id: Ic6882d1452963ca93403748917c313068acfdaab
    Reviewed-on: https://gerrit.openafs.org/14139
    Tested-by: Andrew Deason <adeason@sinenomine.net>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

 src/vlserver/vlprocs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

-- 
OpenAFS Master Repository