vlserver: Correctly pad nvlentry for "O" RPCs 39/14139/2
authorAndrew Deason <adeason@sinenomine.net>
Tue, 7 Apr 2020 18:15:31 +0000 (13:15 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 10 Apr 2020 15:44:48 +0000 (11:44 -0400)
commit7e41ee0bd50d39a356f0435ff370a0a7be40306f
treecca0fbe9a7dcad52a1a3aff31454b2af5aff5d5f
parent30a47c3282cb405459a6fced1fe5b4c77f4afd64
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