afs: Avoid giving wrong 'tf' to afs_InitVolSlot 33/13933/2
authorAndrew Deason <adeason@sinenomine.net>
Tue, 5 Nov 2019 16:50:01 +0000 (10:50 -0600)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 8 Nov 2019 15:54:13 +0000 (10:54 -0500)
commit4a9078c6bbf51720a5eacf7e6ba21443e5103eee
tree489eb182c154b06a2b49932dd5afc9bbcfb06383
parent360b9d5d71fb1de142ae4efd4660732476855a3f
afs: Avoid giving wrong 'tf' to afs_InitVolSlot

Commit 75e3a589 (libafs: afs_InitVolSlot function) split out a bit of
our code that initializes a struct volume into the afs_InitVolSlot
function. However, it caused us to almost always pass a non-NULL 'tf'
to afs_InitVolSlot, even if the target volume was not found.

That is, before that commit, our code roughly did this:

    for (...; j != 0; j = tf->next) {
        ...;
        tf = &staticVolume;
        if (tf->volume == volid)
            break;
    }
    if (tf && j != 0) {
        use_tf_data();
    } else {
        use_blank_data();
    }

The reason for the extra 'j != 0' check after the loop is to see if we
hit the end of the volume hash chain, or if we actually found a
matching 'tf' in the loop.

And after that commit, the code did this:

    for (...; j != 0; j = tf->next) {
        ...;
        if (j != 0) {
            tf = &staticVolume;
            if (tf->volume == volid)
                break;
        }
    }
    if (tf) {
        use_tf_data();
    } else {
        use_blank_data();
    }

The check for 'j != 0' was moved to inside the for loop, but 'j' is
always nonzero in the loop (otherwise, the for() would exit the loop).
This means that if we didn't find a matching 'tf' in the loop, our
'tf' would be non-NULL anyway, and so we'd initialize our volume slot
from just the last entry in the hash chain.

This means that for volumes that are not found in the VolumeItems
file, our struct volume will probably be initialized with arbitrary
data from another volume, instead of being initialized to the normal
defaults (the 'else' clause in afs_InitVolSlot).

This means that the 'dotdot' entry for the volume may be wrong, and so
we may report the wrong parent dir for the root of a volume. However,
the 'dotdot' entry should be fixed when the volume root is accessed
via a mountpoint, so any such issue should be temporary. And of
course, on some platforms (LINUX) we don't ever use the 'dotdot'
information for a volume, and even on other platforms, often resolving
the '..' entry is handled by other means (e.g. shells often calculate
it themselves). But some 'pwd' calculations and other '..' corner
cases may be affected.

To fix this, change the relevant loop so that we only set 'tf' to
non-NULL when we actually find a matching entry.

Change-Id: I53118960462c0057725e749cbf588e98024217c3
Reviewed-on: https://gerrit.openafs.org/13933
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
src/afs/afs_volume.c